Merge pull request #126 from sartography/feature/tenant-specific-fields-from-openid

Feature/tenant specific fields from openid
2023-02-06 16:26:59 -05:00 · 2023-02-06 16:26:59 -05:00 · e9d901b50b
parent c00338e951 0705bf5d58
commit e9d901b50b
13 changed files with 688 additions and 449 deletions
--- a/spiffworkflow-backend/keycloak/bin/add_test_users_to_keycloak
+++ b/spiffworkflow-backend/keycloak/bin/add_test_users_to_keycloak
@ -44,14 +44,66 @@ result=$(curl --fail -s -X POST "$KEYCLOAK_URL" "$INSECURE" \
 )
 backend_token=$(jq -r '.access_token' <<< "$result")

-while read -r user_email; do
-  if [[ -n "$user_email" ]]; then
-    username=$(awk -F '@' '{print $1}' <<<"$user_email")
-    credentials='{"type":"password","value":"'"${username}"'","temporary":false}'
+function add_user() {
+  local user_email=$1
+  local username=$2
+  local user_attribute_one=$3

-    curl --fail --location --request POST "http://localhost:7002/admin/realms/${keycloak_realm}/users" \
-      -H 'Content-Type: application/json' \
-      -H "Authorization: Bearer $backend_token" \
-      --data-raw '{"email":"'"${user_email}"'", "enabled":"true", "username":"'"${username}"'", "credentials":['"${credentials}"']}'
+  local credentials='{"type":"password","value":"'"${username}"'","temporary":false}'
+
+  local data='{"email":"'"${user_email}"'", "enabled":"true", "username":"'"${username}"'", "credentials":['"${credentials}"']'
+  if [[ -n "$user_attribute_one" ]]; then
+    data=''${data}', "attributes": {"'${custom_attribute_one}'": [ "'$user_attribute_one'" ]}'
+  fi
+  data="${data}}"
+
+  local http_code
+  http_code=$(curl --silent -o /dev/null -w '%{http_code}' --location --request POST "http://localhost:7002/admin/realms/${keycloak_realm}/users" \
+    -H 'Content-Type: application/json' \
+    -H "Authorization: Bearer $backend_token" \
+    --data-raw "$data")
+  echo "$http_code"
+}
+
+first_line_processed="false"
+custom_attribute_one=''
+
+while read -r input_line; do
+  if ! grep -qE '^#' <<<"$input_line" ; then
+    if [[ "$first_line_processed" == "false" ]]; then
+      email_header=$(awk -F ',' '{print $1}' <<<"$input_line")
+      if [[ "$email_header" != "email" ]]; then
+        >&2 echo "ERROR: the first column in the first row must be email."
+        exit 1
+      fi
+      custom_attribute_one=$(awk -F ',' '{print $2}' <<<"$input_line")
+      first_line_processed="true"
+    elif [[ -n "$input_line" ]]; then
+      user_email=$(awk -F ',' '{print $1}' <<<"$input_line")
+      username=$(awk -F '@' '{print $1}' <<<"$user_email")
+      user_attribute_one=$(awk -F ',' '{print $2}' <<<"$input_line")
+      http_code=$(add_user "$user_email" "$username" "$user_attribute_one")
+
+      if [[ "$http_code" == "409" ]]; then
+        user_info=$(curl --fail --silent --location --request GET "http://localhost:7002/admin/realms/${keycloak_realm}/users?username=${username}&exact=true" \
+          -H 'Content-Type: application/json' \
+          -H "Authorization: Bearer $backend_token")
+
+        user_id=$(jq -r '.[0] | .id' <<<"$user_info")
+        if [[ -z "$user_id" ]]; then
+          >&2 echo "ERROR: Could not find user_id for user: ${user_email}"
+          exit 1
+        fi
+        curl --fail --location --silent --request DELETE "http://localhost:7002/admin/realms/${keycloak_realm}/users/${user_id}" \
+          -H 'Content-Type: application/json' \
+          -H "Authorization: Bearer $backend_token"
+
+        http_code=$(add_user "$user_email" "$username" "$user_attribute_one")
+        if [[ "$http_code" != "201" ]]; then
+          >&2 echo "ERROR: Failed to recreate user: ${user_email} with http_code: ${http_code}"
+          exit 1
+        fi
+      fi
+    fi
  fi
 done <"$user_file_with_one_email_per_line"
--- a/spiffworkflow-backend/keycloak/realm_exports/spiffworkflow-realm.json
+++ b/spiffworkflow-backend/keycloak/realm_exports/spiffworkflow-realm.json
--- a/spiffworkflow-backend/keycloak/test_user_lists/sartography
+++ b/spiffworkflow-backend/keycloak/test_user_lists/sartography
@ -1,6 +1,7 @@
+email,spiffworkflow-employeeid
 admin@spiffworkflow.org
-alex@sartography.com
-dan@sartography.com
+alex@sartography.com,111
+dan@sartography.com,115
 daniel@sartography.com
 elizabeth@sartography.com
 j@sartography.com
--- a/spiffworkflow-backend/keycloak/test_user_lists/status
+++ b/spiffworkflow-backend/keycloak/test_user_lists/status
@ -1,50 +1,52 @@
+email,spiffworkflow-employeeid
 admin@spiffworkflow.org
 amir@status.im
 app.program.lead@status.im
-core@status.im
+core@status.im,113
 dao.project.lead@status.im
 desktop.program.lead@status.im
 desktop.project.lead@status.im
 fin1@status.im
-fin@status.im
-finance.lead@status.im
-finance.sme@status.im
+fin@status.im,118
+finance.lead@status.im,1182
+finance.lead@status.im,1289
 finance_user1@status.im
-harmeet@status.im
+harmeet@status.im,109
 infra.program-lead@status.im
 infra.project-lead@status.im
-infra.sme@status.im
+infra.sme@status.im,1202
 infra1.sme@status.im
 infra2.sme@status.im
 jakub@status.im
 jarrad@status.im
 lead1@status.im
-lead@status.im
-legal.lead@status.im
+lead@status.im,1140
+legal.lead@status.im,1243
 legal.program-lead.sme@status.im
 legal.program-lead@status.im
 legal.project-lead.sme@status.im
 legal.project-lead@status.im
-legal.sme@status.im
+legal.sme1@status.im,1345
+legal.sme@status.im,1253
 legal1.sme@status.im
-manuchehr@status.im
+manuchehr@status.im,110
 peopleops.partner@status.im
 peopleops.talent.program-lead@status.im
 peopleops.talent.project-lead@status.im
 peopleops.talent.sme@status.im
 peopleops.talent1.sme@status.im
-peopleops.talent@status.im
+peopleops.talent@status.im,141
 ppg.ba.program-lead@status.im
 ppg.ba.project-lead@status.im
-ppg.ba.sme1@status.im
-ppg.ba.sme@status.im
-ppg.ba@status.im
-program.lead@status.im
-sasha@status.im
+ppg.ba.sme1@status.im,1398
+ppg.ba.sme@status.im,1387
+ppg.ba@status.im,1276
+program.lead@status.im,1211
+sasha@status.im,112
 security.program-lead.sme@status.im
 security.program-lead@status.im
 security.project-lead.sme@status.im
 security.project-lead@status.im
-security.sme@status.im
+security.sme@status.im,1230
 security1.sme@status.im
 services.lead@status.im
--- a/spiffworkflow-backend/migrations/versions/ca9b79dde5cc_.py
+++ b/spiffworkflow-backend/migrations/versions/ca9b79dde5cc_.py
@ -0,0 +1,32 @@
+"""empty message
+
+Revision ID: ca9b79dde5cc
+Revises: 2ec4222f0012
+Create Date: 2023-02-03 21:06:56.396816
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = 'ca9b79dde5cc'
+down_revision = '2ec4222f0012'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.add_column('user', sa.Column('tenant_specific_field_1', sa.String(length=255), nullable=True))
+    op.add_column('user', sa.Column('tenant_specific_field_2', sa.String(length=255), nullable=True))
+    op.add_column('user', sa.Column('tenant_specific_field_3', sa.String(length=255), nullable=True))
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.drop_column('user', 'tenant_specific_field_3')
+    op.drop_column('user', 'tenant_specific_field_2')
+    op.drop_column('user', 'tenant_specific_field_1')
+    # ### end Alembic commands ###
--- a/spiffworkflow-backend/src/spiffworkflow_backend/init.py
+++ b/spiffworkflow-backend/src/spiffworkflow_backend/init.py
@ -1,5 +1,6 @@
 """__init__."""
 import os
+import sys
 from typing import Any

 import connexion  # type: ignore
@ -203,6 +204,9 @@ def configure_sentry(app: flask.app.Flask) -> None:
    if sentry_traces_sample_rate is None:
        raise Exception("SENTRY_TRACES_SAMPLE_RATE is not set somehow")

+    # profiling doesn't work on windows, because of an issue like https://github.com/nvdv/vprof/issues/62
+    profiles_sample_rate = 0 if sys.platform.startswith("win") else 1
+
    sentry_sdk.init(
        dsn=app.config.get("SENTRY_DSN"),
        integrations=[
@ -218,8 +222,6 @@ def configure_sentry(app: flask.app.Flask) -> None:
        traces_sample_rate=float(sentry_traces_sample_rate),
        traces_sampler=traces_sampler,
        # The profiles_sample_rate setting is relative to the traces_sample_rate setting.
-        _experiments={
-            "profiles_sample_rate": 1,
-        },
+        _experiments={"profiles_sample_rate": profiles_sample_rate},
        before_send=before_send,
    )
--- a/spiffworkflow-backend/src/spiffworkflow_backend/config/init.py
+++ b/spiffworkflow-backend/src/spiffworkflow_backend/config/init.py
@ -51,6 +51,19 @@ def load_config_file(app: Flask, env_config_module: str) -> None:
            ) from exception


+def _set_up_tenant_specific_fields_as_list_of_strings(app: Flask) -> None:
+    tenant_specific_fields = app.config.get("TENANT_SPECIFIC_FIELDS")
+
+    if tenant_specific_fields is None or tenant_specific_fields == "":
+        app.config["TENANT_SPECIFIC_FIELDS"] = []
+    else:
+        app.config["TENANT_SPECIFIC_FIELDS"] = tenant_specific_fields.split(",")
+        if len(app.config["TENANT_SPECIFIC_FIELDS"]) > 3:
+            raise ConfigurationError(
+                "TENANT_SPECIFIC_FIELDS can have a maximum of 3 fields"
+            )
+
+
 def setup_config(app: Flask) -> None:
    """Setup_config."""
    # ensure the instance folder exists
@ -108,3 +121,4 @@ def setup_config(app: Flask) -> None:

    thread_local_data = threading.local()
    app.config["THREAD_LOCAL_DATA"] = thread_local_data
+    _set_up_tenant_specific_fields_as_list_of_strings(app)
--- a/spiffworkflow-backend/src/spiffworkflow_backend/config/default.py
+++ b/spiffworkflow-backend/src/spiffworkflow_backend/config/default.py
@ -72,7 +72,7 @@ GIT_SSH_PRIVATE_KEY = environ.get("GIT_SSH_PRIVATE_KEY")
 GIT_USERNAME = environ.get("GIT_USERNAME")
 GIT_USER_EMAIL = environ.get("GIT_USER_EMAIL")

-# Datbase Configuration
+# Database Configuration
 SPIFF_DATABASE_TYPE = environ.get(
    "SPIFF_DATABASE_TYPE", default="mysql"
 )  # can also be sqlite, postgres
@ -88,3 +88,8 @@ SYSTEM_NOTIFICATION_PROCESS_MODEL_MESSAGE_ID = environ.get(
 ALLOW_CONFISCATING_LOCK_AFTER_SECONDS = int(
    environ.get("ALLOW_CONFISCATING_LOCK_AFTER_SECONDS", default="600")
 )
+
+# Tenant specific fields is a comma separated list of field names that we will convert to list of strings
+# and store in the user table's tenant_specific_field_n columns. You can have up to three items in this
+# comma-separated list.
+TENANT_SPECIFIC_FIELDS = environ.get("TENANT_SPECIFIC_FIELDS")
--- a/spiffworkflow-backend/src/spiffworkflow_backend/models/user.py
+++ b/spiffworkflow-backend/src/spiffworkflow_backend/models/user.py
@ -34,6 +34,9 @@ class UserModel(SpiffworkflowBaseDBModel):
    service_id = db.Column(db.String(255), nullable=False, unique=False)
    display_name = db.Column(db.String(255))
    email = db.Column(db.String(255))
+    tenant_specific_field_1: str = db.Column(db.String(255))
+    tenant_specific_field_2: str = db.Column(db.String(255))
+    tenant_specific_field_3: str = db.Column(db.String(255))
    updated_at_in_seconds: int = db.Column(db.Integer)
    created_at_in_seconds: int = db.Column(db.Integer)

--- a/spiffworkflow-backend/src/spiffworkflow_backend/scripts/get_current_user.py
+++ b/spiffworkflow-backend/src/spiffworkflow_backend/scripts/get_current_user.py
@ -1,6 +1,7 @@
-"""Get_env."""
+"""Get current user."""
 from typing import Any

+from flask import current_app
 from flask import g

 from spiffworkflow_backend.models.script_attributes_context import (
@ -10,8 +11,6 @@ from spiffworkflow_backend.scripts.script import Script


 class GetCurrentUser(Script):
-    """GetCurrentUser."""
-
    @staticmethod
    def requires_privileged_permissions() -> bool:
        """We have deemed this function safe to run without elevated permissions."""
@ -28,4 +27,7 @@ class GetCurrentUser(Script):
        **kwargs: Any
    ) -> Any:
        """Run."""
-        return g.user.username
+        # dump the user using our json encoder and then load it back up as a dict
+        # to remove unwanted field types
+        user_as_json_string = current_app.json.dumps(g.user)
+        return current_app.json.loads(user_as_json_string)
--- a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py
+++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py
@ -485,38 +485,42 @@ class AuthorizationService:
            .filter(UserModel.service_id == user_info["sub"])
            .first()
        )
-        email = display_name = username = ""
+        user_attributes = {}
+
        if "email" in user_info:
-            username = user_info["email"]
-            email = user_info["email"]
+            user_attributes["username"] = user_info["email"]
+            user_attributes["email"] = user_info["email"]
        else:  # we fall back to the sub, which may be very ugly.
-            username = user_info["sub"] + "@" + user_info["iss"]
+            fallback_username = user_info["sub"] + "@" + user_info["iss"]
+            user_attributes["username"] = fallback_username

        if "preferred_username" in user_info:
-            display_name = user_info["preferred_username"]
+            user_attributes["display_name"] = user_info["preferred_username"]
        elif "nickname" in user_info:
-            display_name = user_info["nickname"]
+            user_attributes["display_name"] = user_info["nickname"]
        elif "name" in user_info:
-            display_name = user_info["name"]
+            user_attributes["display_name"] = user_info["name"]
+
+        user_attributes["service"] = user_info["iss"]
+        user_attributes["service_id"] = user_info["sub"]
+
+        for field_index, tenant_specific_field in enumerate(
+            current_app.config["TENANT_SPECIFIC_FIELDS"]
+        ):
+            if tenant_specific_field in user_info:
+                field_number = field_index + 1
+                user_attributes[f"tenant_specific_field_{field_number}"] = user_info[
+                    tenant_specific_field
+                ]

        if user_model is None:
            current_app.logger.debug("create_user in login_return")
            is_new_user = True
-            user_model = UserService().create_user(
-                username=username,
-                service=user_info["iss"],
-                service_id=user_info["sub"],
-                email=email,
-                display_name=display_name,
-            )
-
+            user_model = UserService().create_user(**user_attributes)
        else:
            # Update with the latest information
-            user_model.username = username
-            user_model.email = email
-            user_model.display_name = display_name
-            user_model.service = user_info["iss"]
-            user_model.service_id = user_info["sub"]
+            for key, value in user_attributes.items():
+                setattr(user_model, key, value)

        # this may eventually get too slow.
        # when it does, be careful about backgrounding, because
--- a/spiffworkflow-backend/tests/data/model_with_lanes/lanes_with_owner_dict.bpmn
+++ b/spiffworkflow-backend/tests/data/model_with_lanes/lanes_with_owner_dict.bpmn
@ -82,7 +82,8 @@
      <bpmn:incoming>Flow_0bgkfue</bpmn:incoming>
      <bpmn:outgoing>Flow_1ivhu7x</bpmn:outgoing>
      <bpmn:script>approver = get_current_user()
-lane_owners["Finance Team"].remove(approver)</bpmn:script>
+username = approver['username']
+lane_owners["Finance Team"].remove(username)</bpmn:script>
    </bpmn:scriptTask>
  </bpmn:process>
  <bpmndi:BPMNDiagram id="BPMNDiagram_1">
--- a/spiffworkflow-backend/tests/spiffworkflow_backend/scripts/test_get_current_user.py
+++ b/spiffworkflow-backend/tests/spiffworkflow_backend/scripts/test_get_current_user.py
@ -0,0 +1,46 @@
+"""Test_get_localtime."""
+import json
+
+from flask import g
+from flask.app import Flask
+from flask.testing import FlaskClient
+from tests.spiffworkflow_backend.helpers.base_test import BaseTest
+
+from spiffworkflow_backend.models.db import db
+from spiffworkflow_backend.models.script_attributes_context import (
+    ScriptAttributesContext,
+)
+from spiffworkflow_backend.models.user import UserModel
+from spiffworkflow_backend.scripts.get_current_user import GetCurrentUser
+
+
+class TestGetCurrentUser(BaseTest):
+    def test_get_current_user(
+        self,
+        app: Flask,
+        client: FlaskClient,
+        with_db_and_bpmn_file_cleanup: None,
+        with_super_admin_user: UserModel,
+    ) -> None:
+        """Test_can_get_members_of_a_group."""
+        testuser1 = self.find_or_create_user("testuser1")
+        testuser1.tenant_specific_field_1 = "456"
+        db.session.add(testuser1)
+        db.session.commit()
+
+        testuser1 = self.find_or_create_user("testuser1")
+        g.user = testuser1
+        process_model_identifier = "test_process_model"
+        process_instance_id = 1
+        script_attributes_context = ScriptAttributesContext(
+            task=None,
+            environment_identifier="testing",
+            process_instance_id=process_instance_id,
+            process_model_identifier=process_model_identifier,
+        )
+        result = GetCurrentUser().run(
+            script_attributes_context,
+        )
+        assert result["username"] == "testuser1"
+        assert result["tenant_specific_field_1"] == "456"
+        json.dumps(result)