From bec5d11bfe05c2e6575c5268b84d9d66a1c3cdb7 Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 18 Mar 2019 16:42:53 -0400 Subject: [PATCH 1/5] users have permission sets for site-wide perms --- ...f7_users_to_permission_sets_join_table_.py | 37 ++++++++++++++ atst/domain/authnid/__init__.py | 4 +- atst/domain/authz.py | 6 +-- atst/domain/portfolios/portfolios.py | 1 - atst/domain/users.py | 21 +++----- atst/models/mixins/__init__.py | 1 + atst/models/mixins/permissions.py | 6 +++ atst/models/portfolio_role.py | 10 ++-- atst/models/user.py | 39 +++++++-------- atst/routes/dev.py | 24 ++++++--- tests/domain/test_audit_log.py | 2 +- tests/domain/test_portfolios.py | 6 +-- tests/domain/test_users.py | 49 ++++++------------- tests/factories.py | 7 ++- tests/routes/portfolios/test_applications.py | 2 +- tests/test_auth.py | 2 +- 16 files changed, 114 insertions(+), 103 deletions(-) create mode 100644 alembic/versions/fc08d99bb7f7_users_to_permission_sets_join_table_.py create mode 100644 atst/models/mixins/permissions.py diff --git a/alembic/versions/fc08d99bb7f7_users_to_permission_sets_join_table_.py b/alembic/versions/fc08d99bb7f7_users_to_permission_sets_join_table_.py new file mode 100644 index 00000000..578af938 --- /dev/null +++ b/alembic/versions/fc08d99bb7f7_users_to_permission_sets_join_table_.py @@ -0,0 +1,37 @@ +"""users to permission_sets join table, remove role rel + +Revision ID: fc08d99bb7f7 +Revises: a19138e386c4 +Create Date: 2019-03-18 06:13:43.128905 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = 'fc08d99bb7f7' +down_revision = 'a19138e386c4' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('users_permission_sets', + sa.Column('user_id', postgresql.UUID(as_uuid=True), nullable=True), + sa.Column('permission_set_id', postgresql.UUID(as_uuid=True), nullable=True), + sa.ForeignKeyConstraint(['permission_set_id'], ['permission_sets.id'], ), + sa.ForeignKeyConstraint(['user_id'], ['users.id'], ) + ) + op.drop_constraint('users_atat_role_id_fkey', 'users', type_='foreignkey') + op.drop_column('users', 'atat_role_id') + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('users', sa.Column('atat_role_id', postgresql.UUID(), autoincrement=False, nullable=True)) + op.create_foreign_key('users_atat_role_id_fkey', 'users', 'permission_sets', ['atat_role_id'], ['id']) + op.drop_table('users_permission_sets') + # ### end Alembic commands ### diff --git a/atst/domain/authnid/__init__.py b/atst/domain/authnid/__init__.py index b644020e..6e43a9f7 100644 --- a/atst/domain/authnid/__init__.py +++ b/atst/domain/authnid/__init__.py @@ -31,9 +31,7 @@ class AuthenticationContext: except NotFoundError: email = self._get_user_email() - return Users.create( - atat_role_name="default", email=email, **self.parsed_sdn - ) + return Users.create(permission_sets=[], email=email, **self.parsed_sdn) def _get_user_email(self): try: diff --git a/atst/domain/authz.py b/atst/domain/authz.py index eae7cf3f..76343ca9 100644 --- a/atst/domain/authz.py +++ b/atst/domain/authz.py @@ -16,7 +16,7 @@ class Authorization(object): @classmethod def has_atat_permission(cls, user, permission): - return permission in user.atat_role.permissions + return permission in user.permissions @classmethod def is_in_portfolio(cls, user, portfolio): @@ -36,10 +36,6 @@ class Authorization(object): def can_view_audit_log(cls, user): return Authorization.has_atat_permission(user, Permissions.VIEW_AUDIT_LOG) - @classmethod - def is_ccpo(cls, user): - return user.atat_role.name == "ccpo" - @classmethod def is_ko(cls, user, task_order): return user == task_order.contracting_officer diff --git a/atst/domain/portfolios/portfolios.py b/atst/domain/portfolios/portfolios.py index 9ce6c2e9..90797eeb 100644 --- a/atst/domain/portfolios/portfolios.py +++ b/atst/domain/portfolios/portfolios.py @@ -100,7 +100,6 @@ class Portfolios(object): first_name=data["first_name"], last_name=data["last_name"], email=data["email"], - atat_role_name="default", provisional=True, ) permission_sets = data.get("permission_sets", []) diff --git a/atst/domain/users.py b/atst/domain/users.py index c91a17b4..21a896a3 100644 --- a/atst/domain/users.py +++ b/atst/domain/users.py @@ -28,11 +28,14 @@ class Users(object): return user @classmethod - def create(cls, dod_id, atat_role_name=None, **kwargs): - atat_role = PermissionSets.get(atat_role_name) + def create(cls, dod_id, permission_sets=None, **kwargs): + if permission_sets: + permission_sets = PermissionSets.get_many(permission_sets) + else: + permission_sets = [] try: - user = User(dod_id=dod_id, atat_role=atat_role, **kwargs) + user = User(dod_id=dod_id, permission_sets=permission_sets, **kwargs) db.session.add(user) db.session.commit() except IntegrityError: @@ -52,18 +55,6 @@ class Users(object): return user - @classmethod - def update_role(cls, user_id, atat_role_name): - - user = Users.get(user_id) - atat_role = PermissionSets.get(atat_role_name) - user.atat_role = atat_role - - db.session.add(user) - db.session.commit() - - return user - _UPDATEABLE_ATTRS = { "first_name", "last_name", diff --git a/atst/models/mixins/__init__.py b/atst/models/mixins/__init__.py index fc0dc51f..ebc2b362 100644 --- a/atst/models/mixins/__init__.py +++ b/atst/models/mixins/__init__.py @@ -1,2 +1,3 @@ from .timestamps import TimestampsMixin from .auditable import AuditableMixin +from .permissions import PermissionsMixin diff --git a/atst/models/mixins/permissions.py b/atst/models/mixins/permissions.py new file mode 100644 index 00000000..62aeb53d --- /dev/null +++ b/atst/models/mixins/permissions.py @@ -0,0 +1,6 @@ +class PermissionsMixin(object): + @property + def permissions(self): + return [ + perm for permset in self.permission_sets for perm in permset.permissions + ] diff --git a/atst/models/portfolio_role.py b/atst/models/portfolio_role.py index eb589157..28d00de6 100644 --- a/atst/models/portfolio_role.py +++ b/atst/models/portfolio_role.py @@ -37,7 +37,9 @@ portfolio_roles_permission_sets = Table( ) -class PortfolioRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): +class PortfolioRole( + Base, mixins.TimestampsMixin, mixins.AuditableMixin, mixins.PermissionsMixin +): __tablename__ = "portfolio_roles" id = Id() @@ -56,12 +58,6 @@ class PortfolioRole(Base, mixins.TimestampsMixin, mixins.AuditableMixin): "PermissionSet", secondary=portfolio_roles_permission_sets ) - @property - def permissions(self): - return [ - perm for permset in self.permission_sets for perm in permset.permissions - ] - def __repr__(self): return "".format( self.portfolio.name, self.user_id, self.id, self.permissions diff --git a/atst/models/user.py b/atst/models/user.py index 709672cc..22806c7d 100644 --- a/atst/models/user.py +++ b/atst/models/user.py @@ -1,4 +1,4 @@ -from sqlalchemy import String, ForeignKey, Column, Date, Boolean +from sqlalchemy import String, ForeignKey, Column, Date, Boolean, Table from sqlalchemy.orm import relationship from sqlalchemy.dialects.postgresql import UUID @@ -6,14 +6,24 @@ from atst.models import Base, types, mixins from atst.models.permissions import Permissions -class User(Base, mixins.TimestampsMixin, mixins.AuditableMixin): +users_permission_sets = Table( + "users_permission_sets", + Base.metadata, + Column("user_id", UUID(as_uuid=True), ForeignKey("users.id")), + Column("permission_set_id", UUID(as_uuid=True), ForeignKey("permission_sets.id")), +) + + +class User( + Base, mixins.TimestampsMixin, mixins.AuditableMixin, mixins.PermissionsMixin +): __tablename__ = "users" id = types.Id() username = Column(String) - atat_role_id = Column(UUID(as_uuid=True), ForeignKey("permission_sets.id")) - atat_role = relationship("PermissionSet") + permission_sets = relationship("PermissionSet", secondary=users_permission_sets) + portfolio_roles = relationship("PortfolioRole", backref="user") email = Column(String, unique=True) @@ -52,36 +62,21 @@ class User(Base, mixins.TimestampsMixin, mixins.AuditableMixin): ] ) - @property - def atat_permissions(self): - return self.atat_role.permissions - - @property - def atat_role_name(self): - return self.atat_role.name - @property def full_name(self): return "{} {}".format(self.first_name, self.last_name) @property def has_portfolios(self): - return ( - Permissions.VIEW_PORTFOLIO in self.atat_role.permissions - ) or self.portfolio_roles + return (Permissions.VIEW_PORTFOLIO in self.permissions) or self.portfolio_roles @property def displayname(self): return self.full_name def __repr__(self): - return "".format( - self.full_name, - self.dod_id, - self.email, - self.atat_role_name, - self.has_portfolios, - self.id, + return "".format( + self.full_name, self.dod_id, self.email, self.has_portfolios, self.id ) def to_dictionary(self): diff --git a/atst/routes/dev.py b/atst/routes/dev.py index ee43a38d..ac444cb1 100644 --- a/atst/routes/dev.py +++ b/atst/routes/dev.py @@ -11,18 +11,33 @@ import pendulum from . import redirect_after_login_url from atst.domain.users import Users +from atst.domain.permission_sets import PermissionSets from atst.queue import queue from tests.factories import random_service_branch from atst.utils import pick bp = Blueprint("dev", __name__) +_ALL_PERMS = [ + "ccpo", + PermissionSets.VIEW_PORTFOLIO, + PermissionSets.VIEW_PORTFOLIO_APPLICATION_MANAGEMENT, + PermissionSets.VIEW_PORTFOLIO_FUNDING, + PermissionSets.VIEW_PORTFOLIO_REPORTS, + PermissionSets.VIEW_PORTFOLIO_ADMIN, + PermissionSets.EDIT_PORTFOLIO_APPLICATION_MANAGEMENT, + PermissionSets.EDIT_PORTFOLIO_FUNDING, + PermissionSets.EDIT_PORTFOLIO_REPORTS, + PermissionSets.EDIT_PORTFOLIO_ADMIN, + PermissionSets.PORTFOLIO_POC, +] + _DEV_USERS = { "sam": { "dod_id": "6346349876", "first_name": "Sam", "last_name": "Stevenson", - "atat_role_name": "ccpo", + "permission_sets": _ALL_PERMS, "email": "sam@example.com", "service_branch": random_service_branch(), "phone_number": "1234567890", @@ -34,7 +49,6 @@ _DEV_USERS = { "dod_id": "2345678901", "first_name": "Amanda", "last_name": "Adamson", - "atat_role_name": "default", "email": "amanda@example.com", "service_branch": random_service_branch(), "phone_number": "1234567890", @@ -46,7 +60,6 @@ _DEV_USERS = { "dod_id": "3456789012", "first_name": "Brandon", "last_name": "Buchannan", - "atat_role_name": "default", "email": "brandon@example.com", "service_branch": random_service_branch(), "phone_number": "1234567890", @@ -58,7 +71,6 @@ _DEV_USERS = { "dod_id": "4567890123", "first_name": "Christina", "last_name": "Collins", - "atat_role_name": "default", "email": "christina@example.com", "service_branch": random_service_branch(), "phone_number": "1234567890", @@ -70,7 +82,6 @@ _DEV_USERS = { "dod_id": "5678901234", "first_name": "Dominick", "last_name": "Domingo", - "atat_role_name": "default", "email": "dominick@example.com", "service_branch": random_service_branch(), "phone_number": "1234567890", @@ -82,7 +93,6 @@ _DEV_USERS = { "dod_id": "6789012345", "first_name": "Erica", "last_name": "Eichner", - "atat_role_name": "default", "email": "erica@example.com", "service_branch": random_service_branch(), "phone_number": "1234567890", @@ -101,7 +111,7 @@ def login_dev(): user_data["dod_id"], **pick( [ - "atat_role_name", + "permission_sets", "first_name", "last_name", "email", diff --git a/tests/domain/test_audit_log.py b/tests/domain/test_audit_log.py index 94128e89..42ff03d3 100644 --- a/tests/domain/test_audit_log.py +++ b/tests/domain/test_audit_log.py @@ -14,7 +14,7 @@ from tests.factories import ( @pytest.fixture(scope="function") def ccpo(): - return UserFactory.from_atat_role("ccpo") + return UserFactory.create_ccpo() @pytest.fixture(scope="function") diff --git a/tests/domain/test_portfolios.py b/tests/domain/test_portfolios.py index dd752416..a0859496 100644 --- a/tests/domain/test_portfolios.py +++ b/tests/domain/test_portfolios.py @@ -151,7 +151,7 @@ def test_owner_can_view_portfolio_members(portfolio, portfolio_owner): @pytest.mark.skip(reason="no ccpo access yet") def test_ccpo_can_view_portfolio_members(portfolio, portfolio_owner): - ccpo = UserFactory.from_atat_role("ccpo") + ccpo = UserFactory.create_ccpo() assert Portfolios.get_with_members(ccpo, portfolio.id) @@ -277,7 +277,7 @@ def test_for_user_does_not_return_inactive_portfolios(portfolio, portfolio_owner @pytest.mark.skip(reason="CCPO status not fully implemented") def test_for_user_returns_all_portfolios_for_ccpo(portfolio, portfolio_owner): - sam = UserFactory.from_atat_role("ccpo") + sam = UserFactory.create_ccpo() PortfolioFactory.create() sams_portfolios = Portfolios.for_user(sam) @@ -297,7 +297,7 @@ def test_get_for_update_information(portfolio, portfolio_owner): assert portfolio == admin_ws # TODO: implement ccpo roles - # ccpo = UserFactory.from_atat_role("ccpo") + # ccpo = UserFactory.create_ccpo() # assert Portfolios.get_for_update_information(ccpo, portfolio.id) developer = UserFactory.create() diff --git a/tests/domain/test_users.py b/tests/domain/test_users.py index 69a83c69..b7030b9c 100644 --- a/tests/domain/test_users.py +++ b/tests/domain/test_users.py @@ -4,81 +4,64 @@ from uuid import uuid4 from atst.domain.users import Users from atst.domain.exceptions import NotFoundError, AlreadyExistsError, UnauthorizedError +from tests.factories import UserFactory + DOD_ID = "my_dod_id" def test_create_user(): - user = Users.create(DOD_ID, "default") - assert user.atat_role.name == "default" + user = Users.create(DOD_ID) + assert user.dod_id == DOD_ID def test_create_user_with_existing_email(): - Users.create(DOD_ID, "default", email="thisusersemail@usersRus.com") + Users.create(DOD_ID, email="thisusersemail@usersRus.com") with pytest.raises(AlreadyExistsError): - Users.create(DOD_ID, "ccpo", email="thisusersemail@usersRus.com") + Users.create(DOD_ID, email="thisusersemail@usersRus.com") +@pytest.mark.skip(reason="no more roles") def test_create_user_with_nonexistent_role(): with pytest.raises(NotFoundError): Users.create(DOD_ID, "nonexistent") def test_get_or_create_nonexistent_user(): - user = Users.get_or_create_by_dod_id(DOD_ID, atat_role_name="default") + user = Users.get_or_create_by_dod_id(DOD_ID) assert user.dod_id == DOD_ID def test_get_or_create_existing_user(): - Users.get_or_create_by_dod_id(DOD_ID, atat_role_name="default") - user = Users.get_or_create_by_dod_id(DOD_ID, atat_role_name="default") - assert user + fact_user = UserFactory.create() + user = Users.get_or_create_by_dod_id(fact_user.dod_id) + assert user == fact_user def test_get_user(): - new_user = Users.create(DOD_ID, "default") + new_user = UserFactory.create() user = Users.get(new_user.id) assert user.id == new_user.id def test_get_nonexistent_user(): - Users.create(DOD_ID, "default") with pytest.raises(NotFoundError): Users.get(uuid4()) def test_get_user_by_dod_id(): - new_user = Users.create(DOD_ID, "default") - user = Users.get_by_dod_id(DOD_ID) + new_user = UserFactory.create() + user = Users.get_by_dod_id(new_user.dod_id) assert user == new_user -def test_update_role(): - new_user = Users.create(DOD_ID, "default") - updated_user = Users.update_role(new_user.id, "ccpo") - - assert updated_user.atat_role.name == "ccpo" - - -def test_update_role_with_nonexistent_user(): - Users.create(DOD_ID, "default") - with pytest.raises(NotFoundError): - Users.update_role(uuid4(), "ccpo") - - -def test_update_existing_user_with_nonexistent_role(): - new_user = Users.create(DOD_ID, "default") - with pytest.raises(NotFoundError): - Users.update_role(new_user.id, "nonexistent") - - def test_update_user(): - new_user = Users.create(DOD_ID, "default") + new_user = UserFactory.create() updated_user = Users.update(new_user, {"first_name": "Jabba"}) assert updated_user.first_name == "Jabba" def test_update_user_with_dod_id(): - new_user = Users.create(DOD_ID, "default") + new_user = UserFactory.create() with pytest.raises(UnauthorizedError) as excinfo: Users.update(new_user, {"dod_id": "1234567890"}) diff --git a/tests/factories.py b/tests/factories.py index bdb27c69..9b1174c4 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -89,7 +89,7 @@ class UserFactory(Base): email = factory.Faker("email") first_name = factory.Faker("first_name") last_name = factory.Faker("last_name") - atat_role = factory.LazyFunction(lambda: PermissionSets.get("default")) + permission_sets = [] dod_id = factory.LazyFunction(random_dod_id) phone_number = factory.LazyFunction(random_phone_number) service_branch = factory.LazyFunction(random_service_branch) @@ -101,9 +101,8 @@ class UserFactory(Base): ) @classmethod - def from_atat_role(cls, atat_role_name, **kwargs): - role = PermissionSets.get(atat_role_name) - return cls.create(atat_role=role, **kwargs) + def create_ccpo(cls, **kwargs): + return cls.create(permission_sets=PermissionSets.get_all(), **kwargs) class PortfolioFactory(Base): diff --git a/tests/routes/portfolios/test_applications.py b/tests/routes/portfolios/test_applications.py index 20e927f6..ff38fd1c 100644 --- a/tests/routes/portfolios/test_applications.py +++ b/tests/routes/portfolios/test_applications.py @@ -42,7 +42,7 @@ def test_user_without_permission_has_no_budget_report_link(client, user_session) @pytest.mark.skip(reason="Temporarily no add activity log link") def test_user_with_permission_has_activity_log_link(client, user_session): portfolio = PortfolioFactory.create() - ccpo = UserFactory.from_atat_role("ccpo") + ccpo = UserFactory.create_ccpo() admin = UserFactory.create() PortfolioRoleFactory.create( portfolio=portfolio, user=admin, status=PortfolioRoleStatus.ACTIVE diff --git a/tests/test_auth.py b/tests/test_auth.py index 39a32ba3..b956137f 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -52,7 +52,7 @@ def test_successful_login_redirect_ccpo(client, monkeypatch): role = PermissionSets.get("ccpo") monkeypatch.setattr( "atst.domain.authnid.AuthenticationContext.get_user", - lambda *args: UserFactory.create(atat_role=role), + lambda *args: UserFactory.create(), ) resp = _login(client) From 366ada5a90edf4d0881d15e9b7a4b19e09c82a21 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 19 Mar 2019 06:06:02 -0400 Subject: [PATCH 2/5] check sitewide perms for portfolio access, restore ccpo tests --- atst/domain/authz.py | 5 ++++- tests/domain/test_audit_log.py | 2 -- tests/domain/test_portfolios.py | 2 -- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/atst/domain/authz.py b/atst/domain/authz.py index 76343ca9..6ee5fa9d 100644 --- a/atst/domain/authz.py +++ b/atst/domain/authz.py @@ -24,7 +24,10 @@ class Authorization(object): @classmethod def check_portfolio_permission(cls, user, portfolio, permission, message): - if not Authorization.has_portfolio_permission(user, portfolio, permission): + if not ( + Authorization.has_atat_permission(user, permission) + or Authorization.has_portfolio_permission(user, portfolio, permission) + ): raise UnauthorizedError(user, message) @classmethod diff --git a/tests/domain/test_audit_log.py b/tests/domain/test_audit_log.py index 42ff03d3..f6bf636c 100644 --- a/tests/domain/test_audit_log.py +++ b/tests/domain/test_audit_log.py @@ -27,7 +27,6 @@ def test_non_admin_cannot_view_audit_log(developer): AuditLog.get_all_events(developer) -@pytest.mark.skip(reason="no ccpo access yet") def test_ccpo_can_view_audit_log(ccpo): events = AuditLog.get_all_events(ccpo) assert len(events) > 0 @@ -42,7 +41,6 @@ def test_paginate_audit_log(ccpo): assert len(events) == 25 -@pytest.mark.skip(reason="no ccpo access yet") def test_ccpo_can_view_ws_audit_log(ccpo): portfolio = PortfolioFactory.create() events = AuditLog.get_portfolio_events(ccpo, portfolio) diff --git a/tests/domain/test_portfolios.py b/tests/domain/test_portfolios.py index a0859496..e429f9b7 100644 --- a/tests/domain/test_portfolios.py +++ b/tests/domain/test_portfolios.py @@ -149,7 +149,6 @@ def test_owner_can_view_portfolio_members(portfolio, portfolio_owner): assert portfolio -@pytest.mark.skip(reason="no ccpo access yet") def test_ccpo_can_view_portfolio_members(portfolio, portfolio_owner): ccpo = UserFactory.create_ccpo() assert Portfolios.get_with_members(ccpo, portfolio.id) @@ -275,7 +274,6 @@ def test_for_user_does_not_return_inactive_portfolios(portfolio, portfolio_owner assert len(bobs_portfolios) == 0 -@pytest.mark.skip(reason="CCPO status not fully implemented") def test_for_user_returns_all_portfolios_for_ccpo(portfolio, portfolio_owner): sam = UserFactory.create_ccpo() PortfolioFactory.create() From 7c5e931c676928fa7007b5e95fedb41bd6c38453 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 19 Mar 2019 06:11:35 -0400 Subject: [PATCH 3/5] throw error for missing permission_sets in PermissionSets.get_many --- atst/domain/permission_sets.py | 7 ++++++- tests/domain/test_permission_sets.py | 5 +++++ tests/domain/test_users.py | 5 ++--- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/atst/domain/permission_sets.py b/atst/domain/permission_sets.py index e78a7cab..1308ec5f 100644 --- a/atst/domain/permission_sets.py +++ b/atst/domain/permission_sets.py @@ -32,12 +32,17 @@ class PermissionSets(object): @classmethod def get_many(cls, perms_set_names): - return ( + permission_sets = ( db.session.query(PermissionSet) .filter(PermissionSet.name.in_(perms_set_names)) .all() ) + if len(permission_sets) != len(perms_set_names): + raise NotFoundError("permission_set") + + return permission_sets + ATAT_ROLES = [ { diff --git a/tests/domain/test_permission_sets.py b/tests/domain/test_permission_sets.py index df19c24d..993cdad4 100644 --- a/tests/domain/test_permission_sets.py +++ b/tests/domain/test_permission_sets.py @@ -30,3 +30,8 @@ def test_get_many(): assert first_or_none( lambda p: p.name == PermissionSets.EDIT_PORTFOLIO_FUNDING, perms_sets ) + + +def test_get_many_nonexistent(): + with pytest.raises(NotFoundError): + PermissionSets.get_many(["nonexistent", "not real"]) diff --git a/tests/domain/test_users.py b/tests/domain/test_users.py index b7030b9c..c83cbd1e 100644 --- a/tests/domain/test_users.py +++ b/tests/domain/test_users.py @@ -20,10 +20,9 @@ def test_create_user_with_existing_email(): Users.create(DOD_ID, email="thisusersemail@usersRus.com") -@pytest.mark.skip(reason="no more roles") -def test_create_user_with_nonexistent_role(): +def test_create_user_with_nonexistent_permission_set(): with pytest.raises(NotFoundError): - Users.create(DOD_ID, "nonexistent") + Users.create(DOD_ID, permission_sets=["nonexistent"]) def test_get_or_create_nonexistent_user(): From 0f9662e2f217d5688bd86c47c93215fc4eac5087 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 19 Mar 2019 06:19:58 -0400 Subject: [PATCH 4/5] restore audit log access test --- tests/domain/test_audit_log.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/tests/domain/test_audit_log.py b/tests/domain/test_audit_log.py index f6bf636c..925e15f2 100644 --- a/tests/domain/test_audit_log.py +++ b/tests/domain/test_audit_log.py @@ -63,17 +63,10 @@ def test_ws_owner_can_view_ws_audit_log(): assert len(events) > 0 -@pytest.mark.skip(reason="all portfolio users can view audit log") -def test_other_users_cannot_view_ws_audit_log(): +def test_other_users_cannot_view_portfolio_audit_log(): with pytest.raises(UnauthorizedError): portfolio = PortfolioFactory.create() dev = UserFactory.create() - PortfolioRoleFactory.create( - portfolio=portfolio, - user=dev, - role=Roles.get("developer"), - status=PortfolioRoleStatus.ACTIVE, - ) AuditLog.get_portfolio_events(dev, portfolio) From c4b4cc091291a22daff14c139dfba7a4f3ab7f52 Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 20 Mar 2019 16:52:32 -0400 Subject: [PATCH 5/5] update atat-wide permission sets --- atst/domain/permission_sets.py | 15 +++++---------- atst/routes/dev.py | 2 +- script/seed_roles.py | 4 ++-- tests/test_auth.py | 2 +- 4 files changed, 9 insertions(+), 14 deletions(-) diff --git a/atst/domain/permission_sets.py b/atst/domain/permission_sets.py index 1308ec5f..99862742 100644 --- a/atst/domain/permission_sets.py +++ b/atst/domain/permission_sets.py @@ -16,6 +16,7 @@ class PermissionSets(object): EDIT_PORTFOLIO_REPORTS = "edit_portfolio_reports" EDIT_PORTFOLIO_ADMIN = "edit_portfolio_admin" PORTFOLIO_POC = "portfolio_poc" + VIEW_AUDIT_LOG = "view_audit_log" @classmethod def get(cls, perms_set_name): @@ -44,19 +45,13 @@ class PermissionSets(object): return permission_sets -ATAT_ROLES = [ +ATAT_PERMISSION_SETS = [ { - "name": "ccpo", - "display_name": "CCPO", + "name": PermissionSets.VIEW_AUDIT_LOG, + "display_name": "View Audit Log", "description": "", "permissions": [Permissions.VIEW_AUDIT_LOG], - }, - { - "name": "default", - "display_name": "Default", - "description": "", - "permissions": [], - }, + } ] _PORTFOLIO_BASIC_PERMISSION_SETS = [ diff --git a/atst/routes/dev.py b/atst/routes/dev.py index ac444cb1..6a9631d5 100644 --- a/atst/routes/dev.py +++ b/atst/routes/dev.py @@ -19,7 +19,6 @@ from atst.utils import pick bp = Blueprint("dev", __name__) _ALL_PERMS = [ - "ccpo", PermissionSets.VIEW_PORTFOLIO, PermissionSets.VIEW_PORTFOLIO_APPLICATION_MANAGEMENT, PermissionSets.VIEW_PORTFOLIO_FUNDING, @@ -30,6 +29,7 @@ _ALL_PERMS = [ PermissionSets.EDIT_PORTFOLIO_REPORTS, PermissionSets.EDIT_PORTFOLIO_ADMIN, PermissionSets.PORTFOLIO_POC, + PermissionSets.VIEW_AUDIT_LOG, ] _DEV_USERS = { diff --git a/script/seed_roles.py b/script/seed_roles.py index 55e69bb0..4340e7a7 100755 --- a/script/seed_roles.py +++ b/script/seed_roles.py @@ -10,11 +10,11 @@ from sqlalchemy.orm.exc import NoResultFound from atst.app import make_config, make_app from atst.database import db from atst.models import PermissionSet, Permissions -from atst.domain.permission_sets import ATAT_ROLES, PORTFOLIO_PERMISSION_SETS +from atst.domain.permission_sets import ATAT_PERMISSION_SETS, PORTFOLIO_PERMISSION_SETS def seed_roles(): - for permission_set_info in ATAT_ROLES + PORTFOLIO_PERMISSION_SETS: + for permission_set_info in ATAT_PERMISSION_SETS + PORTFOLIO_PERMISSION_SETS: permission_set = PermissionSet(**permission_set_info) try: existing_permission_set = ( diff --git a/tests/test_auth.py b/tests/test_auth.py index b956137f..5b95a9d7 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -49,7 +49,7 @@ def test_successful_login_redirect_ccpo(client, monkeypatch): monkeypatch.setattr( "atst.domain.authnid.AuthenticationContext.authenticate", lambda *args: True ) - role = PermissionSets.get("ccpo") + role = PermissionSets.get(PermissionSets.VIEW_AUDIT_LOG) monkeypatch.setattr( "atst.domain.authnid.AuthenticationContext.get_user", lambda *args: UserFactory.create(),