Removing two fields from user table that were not used (uid, name)

Request email from open id clients, as this would provide a handy way to uniquely reference users when assigning to groups.
During Login do a lookup on email if possible -- so that permissions assignments based on email can be connected when sigining in through openid.
Don't use "open_id" for the service name on user accounts, use the iss string provided through open id,  this will allow us to support more than one open id platform.
Update the KeyCloak configuration so it is able to return email addresses for users -- which will make permission assignment easier in the future.
Removed several unused commands in the user_service class.
This commit is contained in:
Dan 2022-12-12 15:43:19 -05:00
parent 14be1fa06b
commit 552229110c
12 changed files with 79 additions and 101 deletions

View File

@ -424,6 +424,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "admin@status.im",
"firstName" : "",
"lastName" : "",
"credentials" : [ {
@ -446,6 +447,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "alex@sartography.com",
"credentials" : [ {
"id" : "81a61a3b-228d-42b3-b39a-f62d8e7f57ca",
"type" : "password",
@ -465,6 +467,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "amir@status.im",
"credentials" : [ {
"id" : "e589f3ad-bf7b-4756-89f7-7894c03c2831",
"type" : "password",
@ -484,6 +487,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "ciadmin1@status.im",
"credentials" : [ {
"id" : "111b5ea1-c2ab-470a-a16b-2373bc94de7a",
"type" : "password",
@ -506,6 +510,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "ciuser1@status.im",
"credentials" : [ {
"id" : "762f36e9-47af-44da-8520-cf09d752497a",
"type" : "password",
@ -528,6 +533,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "core@status.im",
"firstName" : "",
"lastName" : "",
"credentials" : [ {
@ -550,6 +556,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "dan@sartography.com",
"credentials" : [ {
"id" : "d517c520-f500-4542-80e5-7144daef1e32",
"type" : "password",
@ -569,6 +576,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "daniel@sartography.com",
"credentials" : [ {
"id" : "f240495c-265b-42fc-99db-46928580d07d",
"type" : "password",
@ -588,6 +596,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "elizabeth@sartography.com",
"credentials" : [ {
"id" : "ae951ec8-9fc9-4f1b-b340-bbbe463ae5c2",
"type" : "password",
@ -607,6 +616,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "fin@status.im",
"firstName" : "",
"lastName" : "",
"credentials" : [ {
@ -629,6 +639,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "fin1@status.im",
"firstName" : "",
"lastName" : "",
"credentials" : [ {
@ -651,6 +662,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "finance_user1@status.im",
"credentials" : [ {
"id" : "f14722ec-13a7-4d35-a4ec-0475d405ae58",
"type" : "password",
@ -670,6 +682,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "harmeet@status.im",
"credentials" : [ {
"id" : "89c26090-9bd3-46ac-b038-883d02e3f125",
"type" : "password",
@ -689,6 +702,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "j@status.im",
"firstName" : "",
"lastName" : "",
"credentials" : [ {
@ -711,6 +725,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "jakub@status.im",
"credentials" : [ {
"id" : "ce141fa5-b8d5-4bbe-93e7-22e7119f97c2",
"type" : "password",
@ -730,6 +745,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "jarrad@status.im",
"credentials" : [ {
"id" : "113e0343-1069-476d-83f9-21d98edb9cfa",
"type" : "password",
@ -749,6 +765,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "jason@sartography.com",
"credentials" : [ {
"id" : "40abf32e-f0cc-4a17-8231-1a69a02c1b0b",
"type" : "password",
@ -768,6 +785,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "jon@sartography.com",
"credentials" : [ {
"id" : "8b520e01-5b9b-44ab-9ee8-505bd0831a45",
"type" : "password",
@ -787,6 +805,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "kb@sartography.com",
"credentials" : [ {
"id" : "2c0be363-038f-48f1-86d6-91fdd28657cf",
"type" : "password",
@ -806,6 +825,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "lead@status.im",
"firstName" : "",
"lastName" : "",
"credentials" : [ {
@ -828,6 +848,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "lead1@status.im",
"firstName" : "",
"lastName" : "",
"credentials" : [ {
@ -850,6 +871,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "manuchehr@status.im",
"credentials" : [ {
"id" : "07dabf55-b5d3-4f98-abba-3334086ecf5e",
"type" : "password",
@ -869,6 +891,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "mike@sartography.com",
"credentials" : [ {
"id" : "1ed375fb-0f1a-4c2a-9243-2477242cf7bd",
"type" : "password",
@ -888,6 +911,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "natalia@sartography.com",
"credentials" : [ {
"id" : "b6aa9936-39cc-4931-bfeb-60e6753de5ba",
"type" : "password",
@ -907,6 +931,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "sasha@status.im",
"credentials" : [ {
"id" : "4a170af4-6f0c-4e7b-b70c-e674edf619df",
"type" : "password",
@ -926,6 +951,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "service-account@status.im",
"serviceAccountClientId" : "spiffworkflow-backend",
"credentials" : [ ],
"disableableCredentialTypes" : [ ],
@ -943,6 +969,7 @@
"enabled" : true,
"totp" : false,
"emailVerified" : false,
"email": "service-account-withauth@status.im",
"serviceAccountClientId" : "withAuth",
"credentials" : [ ],
"disableableCredentialTypes" : [ ],

View File

@ -1,8 +1,8 @@
"""empty message
Revision ID: 4d75421c0af0
Revision ID: e1d0d593c621
Revises:
Create Date: 2022-12-06 17:42:56.417673
Create Date: 2022-12-12 14:23:44.643766
"""
from alembic import op
@ -10,7 +10,7 @@ import sqlalchemy as sa
# revision identifiers, used by Alembic.
revision = '4d75421c0af0'
revision = 'e1d0d593c621'
down_revision = None
branch_labels = None
depends_on = None
@ -72,14 +72,12 @@ def upgrade():
op.create_table('user',
sa.Column('id', sa.Integer(), nullable=False),
sa.Column('username', sa.String(length=255), nullable=False),
sa.Column('uid', sa.String(length=50), nullable=True),
sa.Column('service', sa.String(length=50), nullable=False),
sa.Column('service_id', sa.String(length=255), nullable=False),
sa.Column('name', sa.String(length=255), nullable=True),
sa.Column('email', sa.String(length=255), nullable=True),
sa.PrimaryKeyConstraint('id'),
sa.UniqueConstraint('service', 'service_id', name='service_key'),
sa.UniqueConstraint('uid')
sa.UniqueConstraint('username')
)
op.create_table('message_correlation_property',
sa.Column('id', sa.Integer(), nullable=False),

View File

@ -63,6 +63,7 @@ groups:
harmeet,
]
# permission "admin"
permissions:
admin:
groups: [admin]
@ -70,6 +71,7 @@ permissions:
allowed_permissions: [create, read, update, delete]
uri: /*
# permission: "basic"
tasks-crud:
groups: [everybody]
users: []
@ -81,7 +83,6 @@ permissions:
allowed_permissions: [read]
uri: /v1.0/service-tasks
# read all for everybody
read-all-process-groups:
groups: [everybody]

View File

@ -2,14 +2,17 @@ default_group: everybody
users:
admin:
service: local_open_id
email: admin@spiffworkflow.org
password: admin
preferred_username: Admin
nelson:
service: local_open_id
email: nelson@spiffworkflow.org
password: nelson
preferred_username: Nelson
malala:
service: local_open_id
email: malala@spiffworkflow.org
password: malala
preferred_username: Malala
@ -72,8 +75,7 @@ permissions:
users: [ ]
allowed_permissions: [ read ]
uri: /v1.0/processes
# Members of the Education group can change they processes work.
# Members of the Education group can change the processes under "education".
education-admin:
groups: ["Education", "President"]
users: []

View File

@ -1,5 +1,12 @@
default_group: everybody
users:
testadmin1:
service: https://testing/openid/thing
email: testadmin1@spiffworkflow.org
password: admin
preferred_username: El administrador de la muerte
groups:
admin:
users: [testadmin1, testadmin2]

View File

@ -28,14 +28,10 @@ class UserModel(SpiffworkflowBaseDBModel):
__tablename__ = "user"
__table_args__ = (db.UniqueConstraint("service", "service_id", name="service_key"),)
id = db.Column(db.Integer, primary_key=True)
# server and service id must be unique, not username.
username = db.Column(db.String(255), nullable=False, unique=False)
uid = db.Column(db.String(50), unique=True)
service = db.Column(db.String(50), nullable=False, unique=False)
username = db.Column(db.String(255), nullable=False, unique=True) # should always be an email address.
service = db.Column(db.String(50), nullable=False, unique=False) # not 'openid' -- google, aws
service_id = db.Column(db.String(255), nullable=False, unique=False)
name = db.Column(db.String(255))
email = db.Column(db.String(255))
user_group_assignments = relationship("UserGroupAssignmentModel", cascade="delete") # type: ignore
@ -47,21 +43,6 @@ class UserModel(SpiffworkflowBaseDBModel):
)
principal = relationship("PrincipalModel", uselist=False) # type: ignore
@validates("service")
def validate_service(self, key: str, value: Any) -> str:
"""Validate_service."""
try:
ap_type = getattr(AuthenticationProviderTypes, value, None)
except Exception as e:
raise ValueError(f"invalid service type: {value}") from e
if ap_type is not None:
ap_value: str = ap_type.value
return ap_value
raise ApiError(
error_code="invalid_service",
message=f"Could not validate service with value: {value}",
)
def encode_auth_token(self) -> str:
"""Generate the Auth Token.

View File

@ -75,7 +75,7 @@ def verify_token(
except ApiError as ae: # API Error is only thrown in the token is outdated.
# Try to refresh the token
user = UserService.get_user_by_service_and_service_id(
"open_id", decoded_token["sub"]
decoded_token["iss"], decoded_token["sub"]
)
if user:
refresh_token = AuthenticationService.get_refresh_token(user.id)
@ -107,7 +107,7 @@ def verify_token(
user_info is not None and "error" not in user_info
): # not sure what to test yet
user_model = (
UserModel.query.filter(UserModel.service == "open_id")
UserModel.query.filter(UserModel.service == user_info["iss"])
.filter(UserModel.service_id == user_info["sub"])
.first()
)
@ -340,9 +340,5 @@ def get_user_from_decoded_internal_token(decoded_token: dict) -> Optional[UserMo
)
if user:
return user
user = UserModel(
username=service_id,
service=service,
service_id=service_id,
)
user = UserService.create_user(service, service_id, username=service_id)
return user

View File

@ -89,7 +89,7 @@ class AuthenticationService:
+ f"?state={state}&"
+ "response_type=code&"
+ f"client_id={self.client_id()}&"
+ "scope=openid&"
+ "scope=openid email&"
+ f"redirect_uri={return_redirect_url}"
)
return login_redirect_url

View File

@ -450,8 +450,13 @@ class AuthorizationService:
def create_user_from_sign_in(cls, user_info: dict) -> UserModel:
"""Create_user_from_sign_in."""
is_new_user = False
if user_info.get('email', None) is not None:
user_model = (
UserModel.query.filter(UserModel.service == "open_id")
UserModel.query.filter(UserModel.email == user_info["email"]).first()
)
else:
user_model = (
UserModel.query.filter(UserModel.service == user_info["iss"])
.filter(UserModel.service_id == user_info["sub"])
.first()
)
@ -459,9 +464,9 @@ class AuthorizationService:
if user_model is None:
current_app.logger.debug("create_user in login_return")
is_new_user = True
name = username = email = ""
username = email = ""
if "name" in user_info:
name = user_info["name"]
username = user_info["name"]
if "username" in user_info:
username = user_info["username"]
elif "preferred_username" in user_info:
@ -469,9 +474,8 @@ class AuthorizationService:
if "email" in user_info:
email = user_info["email"]
user_model = UserService().create_user(
service="open_id",
service=user_info["iss"],
service_id=user_info["sub"],
name=name,
username=username,
email=email,
)

View File

@ -23,7 +23,6 @@ class UserService:
cls,
service: str,
service_id: str,
name: Optional[str] = "",
username: Optional[str] = "",
email: Optional[str] = "",
) -> UserModel:
@ -41,7 +40,6 @@ class UserService:
username=username,
service=service,
service_id=service_id,
name=name,
email=email,
)
db.session.add(user_model)
@ -69,45 +67,12 @@ class UserService:
)
)
@classmethod
def find_or_create_user(
cls,
service: str,
service_id: str,
name: Optional[str] = None,
username: Optional[str] = None,
email: Optional[str] = None,
) -> UserModel:
"""Find_or_create_user."""
user_model: UserModel
try:
user_model = cls.create_user(
service=service,
service_id=service_id,
name=name,
username=username,
email=email,
)
except ApiError:
user_model = (
UserModel.query.filter(UserModel.service == service)
.filter(UserModel.service_id == service_id)
.first()
)
return user_model
# Returns true if the current user is logged in.
@staticmethod
def has_user() -> bool:
"""Has_user."""
return "token" in g and bool(g.token) and "user" in g and bool(g.user)
# Returns true if the given user uid is different from the current user's uid.
@staticmethod
def is_different_user(uid: str) -> bool:
"""Is_different_user."""
return UserService.has_user() and uid is not None and uid is not g.user.uid
@staticmethod
def current_user() -> Any:
"""Current_user."""
@ -117,20 +82,6 @@ class UserService:
)
return g.user
@staticmethod
def in_list(uids: list[str]) -> bool:
"""Returns true if the current user's id is in the given list of ids.
False if there is no user, or the user is not in the list.
"""
if (
UserService.has_user()
): # If someone is logged in, lock tasks that don't belong to them.
user = UserService.current_user()
if user.uid in uids:
return True
return False
@staticmethod
def get_principal_by_user_id(user_id: int) -> PrincipalModel:
"""Get_principal_by_user_id."""

View File

@ -1,4 +1,8 @@
"""Test_authentication."""
import base64
import time
import jwt
from flask import Flask
from flask.testing import FlaskClient
from tests.spiffworkflow_backend.helpers.base_test import BaseTest
@ -44,13 +48,16 @@ class TestFlaskOpenId(BaseTest):
client: FlaskClient,
with_db_and_bpmn_file_cleanup: None,
) -> None:
code = ("testadmin1:1234123412341234")
"""It should be possible to get a token."""
code = (
"c3BpZmZ3b3JrZmxvdy1iYWNrZW5kOkpYZVFFeG0wSmhRUEx1bWdIdElJcWY1MmJEYWxIejBx"
)
backend_basic_auth_string = code
backend_basic_auth_bytes = bytes(backend_basic_auth_string, encoding="ascii")
backend_basic_auth = base64.b64encode(backend_basic_auth_bytes)
headers = {
"Content-Type": "application/x-www-form-urlencoded",
"Authorization": f"Basic {code}",
"Authorization": f"Basic {backend_basic_auth.decode('utf-8')}",
}
data = {
"grant_type": "authorization_code",
@ -59,3 +66,7 @@ class TestFlaskOpenId(BaseTest):
}
response = client.post("/openid/token", data=data, headers=headers)
assert response
assert response.is_json
assert 'access_token' in response.json
assert 'id_token' in response.json
assert 'refresh_token' in response.json

View File

@ -134,7 +134,7 @@ class TestAuthorizationService(BaseTest):
active_task.task_name, processor.bpmn_process_instance
)
finance_user = AuthorizationService.create_user_from_sign_in(
{"username": "testuser2", "sub": "open_id"}
{"username": "testuser2", "sub": "open_id", "iss": "https://test.stuff"}
)
ProcessInstanceService.complete_form_task(
processor, spiff_task, {}, finance_user, active_task