Skip to content

Commit 256ad4b

Browse files
authored
Add user to orgs by default (canada-ca#371)
* Add users to organisations by default This commit creates an org for each user that is created. This org is the default org to which ad hoc scans will be attached. * Add users to an org by default This commit started it's life as a relatively small feature adding a user to an org by default to ensure that users has a usable account after logging in, but ended up with some serious mission creep. Writing tests for this feature uncovered a lack of isolation between tests, and tests that mostly failed with `KeyError` or `'NoneType' object is not subscriptable` errors, as well as test data with clashing ids. This change had some far reaching consequences and I basically kept reworking the tests until the isolation errors went away and I could tell if the "user orgs" feature had actually broken stuff. If it did, I fixed that too.
1 parent 1d2cb8e commit 256ad4b

33 files changed

Lines changed: 2711 additions & 3291 deletions

api/Pipfile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ flask-migrate = "*"
2929
pytest = "*"
3030
pyotp = "*"
3131
pyjwt = "*"
32+
python-slugify = "*"
3233

3334
[scripts]
3435
test = "python -m pytest"

api/Pipfile.lock

Lines changed: 33 additions & 12 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

api/functions/auth_wrappers.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ def check_user_claims(user_claims):
4646
elif user_claims:
4747
user_id = user_claims[0]['user_id']
4848
with app.app_context():
49+
# XXX: affiliations should have been eager loaded with joinedload
50+
# when user was initally pulled from the db.
4951
user_aff = User_affiliations.query.filter(
5052
User_affiliations.user_id == user_id).all()
5153
user_aff = orm_to_dict(user_aff)

api/functions/sign_in_user.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,13 @@ def sign_in_user(user_name, password):
5656
}
5757
user_roles.append(temp_dict)
5858
else:
59+
# XXX: Roles is [] || [""] || [{}]?
60+
# why not just []?
5961
user_roles = ["none"]
6062
try:
6163
payload = {
6264
"exp": datetime.datetime.utcnow()
63-
+ datetime.timedelta(days=0, seconds=1800),
65+
+ datetime.timedelta(days=0, seconds=1800), # XXX: too short
6466
"iat": datetime.datetime.utcnow(),
6567
"user_id": user.id,
6668
"roles": user_roles,
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
"""empty message
2+
3+
Revision ID: b781fb62a115
4+
Revises: 1860746a39e4
5+
Create Date: 2020-05-06 13:19:46.906351
6+
7+
"""
8+
from alembic import op
9+
import sqlalchemy as sa
10+
11+
12+
# revision identifiers, used by Alembic.
13+
revision = "b781fb62a115"
14+
down_revision = "1860746a39e4"
15+
branch_labels = None
16+
depends_on = None
17+
18+
19+
def upgrade():
20+
# ### commands auto generated by Alembic - please adjust! ###
21+
op.add_column("organizations", sa.Column("name", sa.String(), nullable=True))
22+
op.add_column("organizations", sa.Column("slug", sa.String(), nullable=True))
23+
op.create_index(
24+
op.f("ix_organizations_slug"), "organizations", ["slug"], unique=False
25+
)
26+
op.drop_constraint("user_user_aff_key", "user_affiliations", type_="foreignkey")
27+
op.drop_constraint(
28+
"user_affiliations_organization_id_fkey",
29+
"user_affiliations",
30+
type_="foreignkey",
31+
)
32+
op.create_foreign_key(
33+
"user_affiliations_organization_id_fkey",
34+
"user_affiliations",
35+
"organizations",
36+
["organization_id"],
37+
["id"],
38+
onupdate="CASCADE",
39+
ondelete="CASCADE",
40+
)
41+
op.create_foreign_key(
42+
"user_affiliations_users_id_fkey",
43+
"user_affiliations",
44+
"users",
45+
["user_id"],
46+
["id"],
47+
onupdate="CASCADE",
48+
ondelete="CASCADE",
49+
)
50+
# ### end Alembic commands ###
51+
52+
53+
def downgrade():
54+
# ### commands auto generated by Alembic - please adjust! ###
55+
op.drop_constraint(
56+
"user_affiliations_users_id_fkey", "user_affiliations", type_="foreignkey"
57+
)
58+
op.drop_constraint(
59+
"user_affiliations_organization_id_fkey",
60+
"user_affiliations",
61+
type_="foreignkey",
62+
)
63+
op.create_foreign_key(
64+
"user_affiliations_organization_id_fkey",
65+
"user_affiliations",
66+
"organizations",
67+
["organization_id"],
68+
["id"],
69+
)
70+
op.create_foreign_key(
71+
"user_user_aff_key", "user_affiliations", "users", ["user_id"], ["id"]
72+
)
73+
op.drop_index(op.f("ix_organizations_slug"), table_name="organizations")
74+
op.drop_column("organizations", "slug")
75+
op.drop_column("organizations", "name")
76+
# ### end Alembic commands ###
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
"""empty message
2+
3+
Revision ID: 1860746a39e4
4+
Revises: cf42b66dbc12
5+
Create Date: 2020-04-30 22:30:22.681303
6+
7+
"""
8+
from alembic import op
9+
import sqlalchemy as sa
10+
from sqlalchemy.schema import Sequence, CreateSequence
11+
12+
13+
# revision identifiers, used by Alembic.
14+
revision = "1860746a39e4"
15+
down_revision = "cf42b66dbc12"
16+
branch_labels = None
17+
depends_on = None
18+
19+
20+
def upgrade():
21+
op.execute(CreateSequence(Sequence("dmarc_reports_id_seq")))
22+
op.execute(
23+
"ALTER TABLE dmarc_reports ALTER COLUMN id SET DEFAULT nextval('public.dmarc_reports_id_seq')"
24+
)
25+
op.execute("ALTER SEQUENCE dmarc_reports_id_seq OWNED BY dmarc_reports.id")
26+
27+
28+
def downgrade():
29+
op.execute("ALTER TABLE dmarc_reports ALTER COLUMN id DROP DEFAULT")
30+
op.execute("DROP SEQUENCE dmarc_reports_id_seq")

api/models/Organizations.py

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
from graphene import Time
2-
from sqlalchemy.types import Integer, Boolean, DateTime, Float
3-
from sqlalchemy import Column, String, ForeignKey
1+
from sqlalchemy.types import Integer
2+
from sqlalchemy import Column, String
43
from sqlalchemy.orm import relationship
54
from sqlalchemy.dialects.postgresql import JSONB
5+
from slugify import slugify
66
from sqlalchemy.ext.hybrid import hybrid_property, hybrid_method
77
from db import Base
88

@@ -11,11 +11,17 @@ class Organizations(Base):
1111
__tablename__ = "organizations"
1212

1313
id = Column(Integer, primary_key=True)
14+
name = Column(String)
15+
slug = Column(String, index=True)
1416
acronym = Column(String)
1517
org_tags = Column(JSONB)
16-
domains = relationship(
17-
"Domains", back_populates="organization", cascade="all, delete"
18-
)
18+
domains = relationship("Domains", back_populates="organization", cascade="all, delete")
1919
users = relationship(
20-
"User_affiliations", back_populates="user_organization", cascade="all, delete"
20+
"User_affiliations", back_populates="user_organization", passive_deletes=True
2121
)
22+
23+
def __init__(self, **kwargs):
24+
super(Organizations, self).__init__(**kwargs)
25+
self.slug = slugify(kwargs.get("name", ""))
26+
if self.org_tags is None:
27+
self.org_tags = dict()

api/models/User_affiliations.py

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,31 @@ class User_affiliations(Base):
88
__tablename__ = "user_affiliations"
99

1010
id = Column(Integer, primary_key=True, autoincrement=True)
11-
user_id = Column(Integer, ForeignKey("users.id"), primary_key=True)
12-
organization_id = Column(Integer, ForeignKey("organizations.id"))
11+
user_id = Column(
12+
Integer,
13+
ForeignKey(
14+
"users.id",
15+
onupdate="CASCADE",
16+
ondelete="CASCADE",
17+
# name is required on FK or rollback is broken
18+
name="user_affiliations_users_id_fkey",
19+
),
20+
primary_key=True,
21+
)
22+
organization_id = Column(
23+
Integer,
24+
ForeignKey(
25+
"organizations.id",
26+
onupdate="CASCADE",
27+
ondelete="CASCADE",
28+
# name is required on FK or rollback is broken
29+
name="user_affiliations_organization_id_fkey",
30+
),
31+
)
1332
permission = Column(String)
1433
user = relationship(
1534
"Users", back_populates="user_affiliation", cascade="all, delete"
1635
)
1736
user_organization = relationship(
18-
"Organizations", back_populates="users", cascade="all, delete"
37+
"Organizations", back_populates="users", cascade="all, delete",
1938
)

api/models/Users.py

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
11
from sqlalchemy.types import Integer, Boolean, Float
2+
from slugify import slugify
23
from functions.orm_to_dict import orm_to_dict
34
from sqlalchemy import Column, String, ForeignKey
45
from sqlalchemy import event
56
from app import bcrypt
7+
from models.Organizations import Organizations
8+
from models.User_affiliations import User_affiliations
69
from sqlalchemy.orm import relationship, validates
10+
from sqlalchemy.ext.associationproxy import association_proxy
711
from sqlalchemy.ext.hybrid import hybrid_property, hybrid_method
812
from db import Base
913

@@ -20,9 +24,24 @@ class Users(Base):
2024
failed_login_attempt_time = Column(Float, default=0, nullable=True)
2125
tfa_validated = Column(Boolean, default=False)
2226
user_affiliation = relationship(
23-
"User_affiliations", back_populates="user", cascade="all, delete"
27+
"User_affiliations", back_populates="user", passive_deletes=True,
2428
)
2529

30+
def __init__(self, **kwargs):
31+
super(Users, self).__init__(**kwargs)
32+
# XXX: This is gross but matches the expections of the
33+
# Acronym scalar type.
34+
acronym = slugify(self.user_name).upper()[:10]
35+
self.user_affiliation.append(
36+
User_affiliations(
37+
permission="admin",
38+
user_organization=Organizations(
39+
name=self.user_name,
40+
acronym=acronym,
41+
)
42+
)
43+
)
44+
2645
@hybrid_method
2746
def find_by_user_name(self, user_name):
2847
return self.query.filter(self.user_name == user_name).first()

api/models/__init__.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
class Domains(Base):
1313
__tablename__ = "domains"
1414

15-
id = Column(Integer, primary_key=True)
15+
id = Column(Integer, primary_key=True, autoincrement=True)
1616
domain = Column(String)
1717
last_run = Column(DateTime)
1818
organization_id = Column(Integer, ForeignKey("organizations.id"))
@@ -41,7 +41,7 @@ class Dmarc_Reports(Base):
4141
class Scans(Base):
4242
__tablename__ = "scans"
4343

44-
id = Column(Integer, primary_key=True)
44+
id = Column(Integer, primary_key=True, autoincrement=True)
4545
domain_id = Column(Integer, ForeignKey("domains.id"))
4646
scan_date = Column(DateTime)
4747
initiated_by = Column(Integer, ForeignKey("users.id"))

0 commit comments

Comments
 (0)