Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: SSO Migration - DESENG #408 #2333

Merged
merged 7 commits into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions CHANGELOG.MD
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,20 @@

All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](https://semver.org/).

## v1.1.0 - 2023-11-06
> **Feature**: Switch MET to use Keycloak SSO service - [🎟️DESENG-408](https://apps.itsm.gov.bc.ca/jira/browse/DESENG-408)
> - Switch all role-based checks on the API to use a single callback function (`current_app.config['JWT_ROLE_CALLBACK']`)
> - Added a configurable path `JWT_ROLE_CLAIM` to indicate where your SSO instance places role information in the JWT token. If your access token looks like:
> `{ ..., "realm_access": { "roles": [ "role1", "role2"]}}` you would set `JWT_ROLE_CLAIM=realm_access.roles`
> - Explicitly disable single tenant mode to ensure correct multi-tenancy behaviour
> - Remove local Keycloak instances and configuration
> - *Potentially breaking*: Default to the "standard" realm for Keycloak
> - *Potentially breaking*: Use tenancy information from DB rather than Keycloak

> **Feature**: .env var audit and cleanup - [🎟️DESENG-414](https://apps.itsm.gov.bc.ca/jira/browse/DESENG-414)
> - Update sample.env files to properly reflect the current state of the project and document where to download credentials securely
> *This ticket is not closed by this version*


## v1.0.1 - 2023-10-26

Expand Down
65 changes: 0 additions & 65 deletions met-api/docker-compose.yml

This file was deleted.

28 changes: 19 additions & 9 deletions met-api/sample.env
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,24 @@ DATABASE_NAME="met"
# Email API endpoint
NOTIFICATIONS_EMAIL_ENDPOINT=https://met-notify-api-dev.apps.gold.devops.gov.bc.ca/api/v1/notifications/email

# Keycloak configuration. Keycloak is now hosted, and local keycloak instances are no longer needed.
KEYCLOAK_BASE_URL=https://dev.loginproxy.gov.bc.ca/auth
KEYCLOAK_REALMNAME=standard
JWT_OIDC_AUDIENCE=modern-engagement-tools-4787
# Keycloak configuration.
# Populate from 'GDX Modern Engagement Tools-installation-*.json'
# https://bcgov.github.io/sso-requests
KEYCLOAK_BASE_URL= # auth-server-url
KEYCLOAK_REALMNAME= # realm
MET_ADMIN_CLIENT_ID= # resource
MET_ADMIN_CLIENT_SECRET= # credentials.secret

# Copy from 'GDX MET web (public)-installation-*.json'
JWT_OIDC_AUDIENCE= # resource
JWT_OIDC_WELL_KNOWN_CONFIG=${KEYCLOAK_BASE_URL}/realms/${KEYCLOAK_REALMNAME}/.well-known/openid-configuration
JWT_OIDC_JWKS_URI=${KEYCLOAK_BASE_URL}/realms/${KEYCLOAK_REALMNAME}/protocol/openid-connect/certs
JWT_OIDC_ISSUER=${KEYCLOAK_BASE_URL}/realms/${KEYCLOAK_REALMNAME}
# Where Keycloak provides the roles that a user has
JWT_ROLE_CLAIM=realm_access.roles
# JWT_ROLE_CLAIM=client_roles


# Authenticates the MET API with Keycloak for running tests.
# Currently unused since the hosted Keycloak instance does not support API usage.
MET_ADMIN_CLIENT_ID=
MET_ADMIN_CLIENT_SECRET=

# S3 configuration. Used for uploading custom header images, etc.
S3_ACCESS_KEY_ID=
Expand All @@ -42,4 +48,8 @@ EPIC_KEYCLOAK_SERVICE_ACCOUNT_ID=
EPIC_KEYCLOAK_SERVICE_ACCOUNT_SECRET=

# Allowed CORS origins
CORS_ORIGIN=http://localhost:3000,http://localhost:5000
CORS_ORIGIN=http://localhost:3000,http://localhost:5000

# Whether to skip certain auth checks. Should be false in production.
# Must match the value set for REACT_APP_IS_SINGLE_TENANT_ENVIRONMENT in the client app.
IS_SINGLE_TENANT_ENVIRONMENT=false
13 changes: 10 additions & 3 deletions met-api/src/met_api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,15 @@ def build_cache(app):
def setup_jwt_manager(app_context, jwt_manager):
"""Use flask app to configure the JWTManager to work for a particular Realm."""

def get_roles(a_dict):
return a_dict['realm_access']['roles'] # pragma: no cover

def get_roles(token_info):
"""
Consumes a token_info dictionary and returns a list of roles.
Uses a configurable path to the roles in the token_info dictionary.
"""
role_access_path = app_context.config['JWT_ROLE_CLAIM']
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice touch with the customizable path. Will be nice to have while we're in this transition period between auth providers.

for key in role_access_path.split('.'):
token_info = token_info.get(key, {})
return token_info

app_context.config['JWT_ROLE_CALLBACK'] = get_roles
jwt_manager.init_app(app_context)
4 changes: 3 additions & 1 deletion met-api/src/met_api/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ class _Config(): # pylint: disable=too-few-public-methods
JWT_OIDC_CACHING_ENABLED = os.getenv('JWT_OIDC_CACHING_ENABLED', 'True')
JWT_OIDC_JWKS_CACHE_TIMEOUT = 300

JWT_ROLE_CLAIM = os.getenv('JWT_ROLE_CLAIM', 'realm_access.roles')

S3_CONFIG = {
'DEFAULT': {
'S3_BUCKET': os.getenv('S3_BUCKET'),
Expand All @@ -135,7 +137,7 @@ class _Config(): # pylint: disable=too-few-public-methods

# Service account details
KEYCLOAK_BASE_URL = os.getenv('KEYCLOAK_BASE_URL')
KEYCLOAK_REALMNAME = os.getenv('KEYCLOAK_REALMNAME', 'met')
KEYCLOAK_REALMNAME = os.getenv('KEYCLOAK_REALMNAME', 'standard')
KEYCLOAK_SERVICE_ACCOUNT_ID = os.getenv('MET_ADMIN_CLIENT_ID')
KEYCLOAK_SERVICE_ACCOUNT_SECRET = os.getenv('MET_ADMIN_CLIENT_SECRET')
# TODO separate out clients for APIs and user management.
Expand Down
10 changes: 2 additions & 8 deletions met-api/src/met_api/models/base_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,10 @@ def flush(self):
db.session.flush()
return self

def add_to_session(self):
"""Save and flush."""
return self.flush()

def save(self):
"""Save and commit."""
self._set_tenant_id()
db.session.add(self)
db.session.flush()
db.session.commit()
self.flush()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does self access db.session in this instance?

Copy link
Contributor Author

@NatSquared NatSquared Nov 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first argument to a method of a class (called self by convention) always receives a reference to the class itself (in this case, BaseModel). So self.flush() is the same as BaseModel.flush(), which behind the scenes gets called as something like BaseModel.flush(self=BaseModel)

self.commit()

def _set_tenant_id(self):
# add tenant id to the model if the child model has tenant id column
Expand Down
7 changes: 4 additions & 3 deletions met-api/src/met_api/services/authorization.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,13 @@ def check_auth(**kwargs):
"""Check if user is authorized to perform action on the service."""
skip_tenant_check = current_app.config.get('IS_SINGLE_TENANT_ENVIRONMENT')
user_from_context: UserContext = kwargs['user_context']
user_from_db = StaffUserModel.get_user_by_external_id(user_from_context.sub)
token_roles = set(user_from_context.roles)
permitted_roles = set(kwargs.get('one_of_roles', []))
has_valid_roles = token_roles & permitted_roles
if has_valid_roles:
if not skip_tenant_check:
user_tenant_id = user_from_context.tenant_id
user_tenant_id = user_from_db.tenant_id
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, you switched accessing the "current" user from app context to pulling that user from the DB. Was the former solution causing issues?

_validate_tenant(kwargs.get('engagement_id'), user_tenant_id)
return

Expand All @@ -47,8 +48,8 @@ def _validate_tenant(eng_id, tenant_id):
return
engagement_tenant_id = EngagementModel.find_tenant_id_by_id(eng_id)
if engagement_tenant_id and str(tenant_id) != str(engagement_tenant_id):
current_app.logger.debug(f'Aborting . Tenant Id on Engagement and user context Mismatch'
f'engagement_tenant_id:{engagement_tenant_id} '
current_app.logger.debug(f'Aborting . Tenant Id on Engagement and user context Mismatch\n'
f'engagement_tenant_id:{engagement_tenant_id}\n'
f'tenant_id: {tenant_id}')

abort(HTTPStatus.FORBIDDEN)
Expand Down
15 changes: 7 additions & 8 deletions met-api/src/met_api/utils/tenant_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from met_api.auth import jwt as _jwt
from met_api.utils.constants import TENANT_ID_JWT_CLAIM
from met_api.utils.roles import Role
from met_api.models.staff_user import StaffUser


def require_role(role, skip_tenant_check_for_admin=False):
Expand Down Expand Up @@ -54,14 +55,14 @@ def wrapper(*args, **kwargs):
if skip_tenant_check_for_admin and is_met_global_admin(token_info):
return func(*args, **kwargs)

tenant_id = token_info.get(TENANT_ID_JWT_CLAIM, None)
current_app.logger.debug(f'Tenant Id From JWT Claim {tenant_id}')
current_app.logger.debug(f'Tenant Id From g {g.tenant_id}')
if g.tenant_id and str(g.tenant_id) == str(tenant_id):
user_id = token_info.get('sub', None)
# fetch user from the db
user = StaffUser.get_user_by_external_id(user_id)
if user and user.tenant_id == g.tenant_id:
return func(*args, **kwargs)
else:
abort(HTTPStatus.FORBIDDEN,
description='The user has no access to this tenant')
description='The user does not exist or has no access to this tenant')

return wrapper

Expand All @@ -74,7 +75,5 @@ def _get_token_info() -> Dict:

def is_met_global_admin(token_info) -> bool:
"""Return True if the user is MET Admin ie who can manage all tenants."""
roles: list = token_info.get('realm_access', None).get('roles', []) if 'realm_access' in token_info \
else []

roles = current_app.config['JWT_ROLE_CALLBACK'](token_info)
return Role.CREATE_TENANT.value in roles
5 changes: 2 additions & 3 deletions met-api/src/met_api/utils/user_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import functools
from typing import Dict

from flask import g, request
from flask import g, request, current_app

from met_api.utils.constants import TENANT_ID_JWT_CLAIM
from met_api.utils.roles import Role
Expand All @@ -39,8 +39,7 @@ def __init__(self):
self._last_name: str = token_info.get('lastname', None)
self._tenant_id: str = token_info.get(TENANT_ID_JWT_CLAIM, None)
self._bearer_token: str = _get_token()
self._roles: list = token_info.get('realm_access', None).get('roles', []) if 'realm_access' in token_info \
else []
self._roles: list = current_app.config['JWT_ROLE_CALLBACK'](token_info)
self._sub: str = token_info.get('sub', None)
self._name: str = f"{token_info.get('firstname', None)} {token_info.get('lastname', None)}"

Expand Down
18 changes: 12 additions & 6 deletions met-web/sample.env
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
# Keycloak auth endpoint
REACT_APP_KEYCLOAK_URL=https://dev.loginproxy.gov.bc.ca/auth
REACT_APP_KEYCLOAK_REALM=standard

# Resource identifier for the Keycloak client
REACT_APP_KEYCLOAK_CLIENT=modern-engagement-tools-4787
# Keycloak auth
# Copy from 'GDX MET web (public)-installation-*.json'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice helpful notes to aid in gathering those env variables

# https://bcgov.github.io/sso-requests
REACT_APP_KEYCLOAK_URL= # auth-server-url
REACT_APP_KEYCLOAK_REALM= # realm
REACT_APP_KEYCLOAK_CLIENT= # resource

# The role needed to be considered an admin
# TODO: Allocate a dedicated role for this on SSO
Expand All @@ -14,5 +15,10 @@ REACT_APP_API_URL=http://localhost:5000/api

# `analytics-api` endpoint
REACT_APP_ANALYTICS_API_URL=http://localhost:5001/api

# Default tenant to assign when signing in for the first time
REACT_APP_DEFAULT_TENANT=eao
REACT_APP_DEFAULT_TENANT=eao

# Whether to skip certain auth checks. Should be false in production.
# Must match the value set for IS_SINGLE_TENANT_ENVIRONMENT in the API.
REACT_APP_IS_SINGLE_TENANT_ENVIRONMENT=false
22 changes: 0 additions & 22 deletions tools/keycloak/docker-compose.yml

This file was deleted.

Loading