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

Conversation

NatSquared
Copy link
Contributor

@NatSquared NatSquared commented Nov 6, 2023

Issue #: https://apps.itsm.gov.bc.ca/jira/browse/DESENG-408

Description of changes:

v1.1.0 - 2023-11-06

Feature: Switch MET to use Keycloak SSO service - 🎟️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

  • 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

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the met-public license (Apache 2.0).

@NatSquared NatSquared requested review from a team and kshapka-bcgov and removed request for a team November 6, 2023 22:55
@NatSquared NatSquared requested review from jareth-whitney and a team November 6, 2023 22:56
Copy link

sonarqubecloud bot commented Nov 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

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.

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)

# 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

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?

@NatSquared NatSquared merged commit 585def0 into gdx-sso Nov 7, 2023
2 checks passed
@Baelx
Copy link
Collaborator

Baelx commented Nov 7, 2023

APPROVED: Nice work on this! I especially appreciate the comments and detailed changelog and PR notes. Looks good :)

@NatSquared NatSquared deleted the feature/deseng-408-sso-migration branch December 1, 2023 02:19
@NatSquared NatSquared restored the feature/deseng-408-sso-migration branch December 1, 2023 02:19
@NatSquared NatSquared deleted the feature/deseng-408-sso-migration branch December 11, 2023 18:20
NatSquared added a commit that referenced this pull request Dec 11, 2023
* Make role checks platform-agnostic

* default to standard realm

* Remove local Keycloak instances and config

* Use tenant information from DB instead of Keycloak

* Update sample.env files

* Clean up the changelog... and we're good to go! 🥳
NatSquared added a commit that referenced this pull request Dec 11, 2023
* Make role checks platform-agnostic

* default to standard realm

* Remove local Keycloak instances and config

* Use tenant information from DB instead of Keycloak

* Update sample.env files

* Clean up the changelog... and we're good to go! 🥳
NatSquared added a commit that referenced this pull request Dec 11, 2023
* Feature: SSO Migration - DESENG #408 (#2333)

* Make role checks platform-agnostic

* default to standard realm

* Remove local Keycloak instances and config

* Use tenant information from DB instead of Keycloak

* Update sample.env files

* Clean up the changelog... and we're good to go! 🥳

* [DESENG-414] .env var (and config) audit and cleanup (#2339)

* Overhaul of most configuration files

* No longer using semver; update CHANGELOG.MD

* Feature/deseng415 (#2334)

* feature/deseng415: Added recording of date with feedback submission and displaying the data on admin side.

* feature/deseng415: Fixed feedback schema, removed yup import, fixed change log date.

* bugfix/deseng429: Removed outdated service class. (#2337)

* bugfix/deseng429: Removed outdated service class.

* bugfix/deseng429: Changed version and changelog to match deployments to gdx-main.

* DESENG-438 Superusers can publish engagements without attached surveys (#2338)

* DESENG-441 Remove unused engagement metadata fields (#2340)

* Fixed merge errors in the changelog

---------

Co-authored-by: jareth-whitney <110929259+jareth-whitney@users.noreply.github.com>
Co-authored-by: Baelx <16845197+Baelx@users.noreply.github.com>
Co-authored-by: Alex <awintschel@gmail.com>
Well done team! 💖
NatSquared added a commit that referenced this pull request Dec 11, 2023
* Bugfix/deseng413 (#2330)

* bugfix/deseng413: Upgraded BC-Sans font to newest version.

* bugfix/deseng413: Small update to changelog for clarification.

* Feature: SSO Migration - DESENG #408 (#2333)

* Make role checks platform-agnostic

* default to standard realm

* Remove local Keycloak instances and config

* Use tenant information from DB instead of Keycloak

* Update sample.env files

* Clean up the changelog... and we're good to go! 🥳

* [DESENG-414] .env var (and config) audit and cleanup (#2339)

* Overhaul of most configuration files

* No longer using semver; update CHANGELOG.MD

* Feature/deseng415 (#2334)

* feature/deseng415: Added recording of date with feedback submission and displaying the data on admin side.

* feature/deseng415: Fixed feedback schema, removed yup import, fixed change log date.

* bugfix/deseng429: Removed outdated service class. (#2337)

* bugfix/deseng429: Removed outdated service class.

* bugfix/deseng429: Changed version and changelog to match deployments to gdx-main.

* DESENG-438 Superusers can publish engagements without attached surveys (#2338)

* DESENG-441 Remove unused engagement metadata fields (#2340)

* Fixed merge errors in the changelog

---------

Co-authored-by: jareth-whitney <110929259+jareth-whitney@users.noreply.github.com>
Co-authored-by: Baelx <16845197+Baelx@users.noreply.github.com>
Co-authored-by: Alex <awintschel@gmail.com>
Well done team! 💖
NatSquared added a commit that referenced this pull request Dec 12, 2023
* Add initial version of change log (#2318)

* Feature/update sample env files (#2320)

* Remove old production .env file

* Update DEVELOPMENT.md to reflect project state

* Update CHANGELOG.md before PR

* Link JIRA ticket # on relevant changes

* Bring bugfixes from main into gdx-dev (#2328)

* Made slug url case insensitive , Fixed bug with wrong query join for submission (#2321)

* CSV export made working for multipage wizard surveys (#2322)

---------

Co-authored-by: saravanpa-aot <saravankumar.pa@aot-technologies.com>

* bugfix/deseng421: Changed engagement links so that they open in the same window/tab as opposed to a new one. (#2329)

* Merge SSO and dev changes into gdx-main (#2343)

* Bugfix/deseng413 (#2330)

* bugfix/deseng413: Upgraded BC-Sans font to newest version.

* bugfix/deseng413: Small update to changelog for clarification.

* Feature: SSO Migration - DESENG #408 (#2333)

* Make role checks platform-agnostic

* default to standard realm

* Remove local Keycloak instances and config

* Use tenant information from DB instead of Keycloak

* Update sample.env files

* Clean up the changelog... and we're good to go! 🥳

* [DESENG-414] .env var (and config) audit and cleanup (#2339)

* Overhaul of most configuration files

* No longer using semver; update CHANGELOG.MD

* Feature/deseng415 (#2334)

* feature/deseng415: Added recording of date with feedback submission and displaying the data on admin side.

* feature/deseng415: Fixed feedback schema, removed yup import, fixed change log date.

* bugfix/deseng429: Removed outdated service class. (#2337)

* bugfix/deseng429: Changed version and changelog to match deployments to gdx-main.

* DESENG-438 Superusers can publish engagements without attached surveys (#2338)

* DESENG-441 Remove unused engagement metadata fields (#2340)

* Fixed merge errors in the changelog

---------
Co-authored-by: saravanpa-aot <saravankumar.pa@aot-technologies.com>
Co-authored-by: jareth-whitney <110929259+jareth-whitney@users.noreply.github.com>
Co-authored-by: Baelx <16845197+Baelx@users.noreply.github.com>
Co-authored-by: Alex <awintschel@gmail.com>
Thank you, everyone!
---------
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants