Spike: auth sharing with reporting component (MAV-1406) #3866
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
benilovj
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
aldavidson
force-pushed
the
spike/MAV-1406-auth-sharing-with-reporting
branch
5 times, most recently
from
ce2131c to
044f3f7
Compare
aldavidson
force-pushed
the
spike/MAV-1406-auth-sharing-with-reporting
branch
2 times, most recently
from
21de1a5 to
06e6d4d
Compare
aldavidson
force-pushed
the
spike/MAV-1406-auth-sharing-with-reporting
branch
from
6a681a6 to
927d4fe
Compare
aldavidson
changed the title
aldavidson
force-pushed
the
spike/MAV-1406-auth-sharing-with-reporting
branch
from
c882d9d to
9fd9269
Compare
aldavidson
force-pushed
the
spike/MAV-1406-auth-sharing-with-reporting
branch
from
7250628 to
c80e29a
Compare
aldavidson
force-pushed
the
spike/MAV-1406-auth-sharing-with-reporting
branch
from
f02fe88 to
380a216
Compare
misaka
reviewed
Jul 24, 2025
aldavidson
force-pushed
the
spike/MAV-1406-auth-sharing-with-reporting
branch
from
ab78768 to
0d1a7fd
Compare
aldavidson
temporarily deployed
to
sandbox-alpha
GitHub Actions
Inactive
aldavidson
temporarily deployed
to
sandbox-alpha
GitHub Actions
Inactive
aldavidson
temporarily deployed
to
sandbox-alpha
GitHub Actions
Inactive
aldavidson
temporarily deployed
to
sandbox-alpha
GitHub Actions
Inactive
aldavidson
temporarily deployed
to
sandbox-alpha
GitHub Actions
Inactive
aldavidson
force-pushed
the
spike/MAV-1406-auth-sharing-with-reporting
branch
from
3efbf98 to
f904bfc
Compare
aldavidson
force-pushed
the
spike/MAV-1406-auth-sharing-with-reporting
branch
from
f904bfc to
5abd017
Compare
This was referenced Aug 13, 2025
aldavidson
force-pushed
the
spike/MAV-1406-auth-sharing-with-reporting
branch
4 times, most recently
from
11de823 to
beaebe5
Compare
thomasleese
force-pushed
the
next
branch
2 times, most recently
from
349361b to
fe4eabf
Compare
aldavidson
force-pushed
the
spike/MAV-1406-auth-sharing-with-reporting
branch
2 times, most recently
from
8c5149f to
ad27533
Compare
When using CIS2, it doesn't make sense (and is potentially misleading) to give users a fallback role since it won't be being used.
This fixes a bug where column coutns were incorrect due to - counting after importing records - parent realtionships being set on the changeset even when parents aren't present
…l login via CIS2, and specs for same. Add one-time-token exchange with the reporting app. Can now demonstrate logging in on Mavis & redirecting back to reporting app with session info Add JWT to encode all the info that is currently top-level in the token verification response Add reporting controller + spec Add pwd_auth_session_token to the user model. Add Warden callbacks to set & clear it on pwd-login and logout. Make authenticate_user_by_jwt! look for user with not just the given user ID, but also the given session_token and pwd_auth_session_token, return 401 if no match Add specs for authentication-related methods Refactor token auth methods to their own concern Delete pwd_auth_session_token on logout also Remove now-redundant user info from one_time_tokens_controller response - the JWT has all this info in it already Closer alignment with OAUTH2.0 spec Change the GET /tokens/:token endpoint to POST /tokens/verify, to make it more aligned with the OAUTH2.0 spec Add a separate secret value - CLIENT_ID Use CLIENT_ID to authorize the client when exchanging a one-time-token for an Access Token, as per the spec Add required grant_type param to the Access Token request Make the Access Token request a POST, and use a more appropriate URL for it (/tokens/verify) Use the request body to pass params to this request, rather than headers - as per the spec Remove authorization by param/header feature flags, replace them with a single reporting_app Rename token to code and redirect_after_login to redirect_uri, to align more closely with the OAUTH 2.0 spec Rename route /tokens/verify -> /tokens/authorize (better-aligned with the OAuth spec)) Rename pwd_auth_session_token to reporting_app_session_token Explicitly set and clear reporting_app_session_token on login/logout regardless of login mechanism Add PlantUML diagrams for auth flow between Mavis and the reporting app Add document including the diagrams of auth flow between Mavis and the reporting app Add user visiting the reporting app diagram to auth flow adoc re-scope the reporting API controller to be under /reporting-api/, and inherit a base_controller which handles the auth and ensures that reporting_app feature flag is enabled. Fixes MAV-1558 and MAV-1588 CodeQL fixes - dont skip verify_authenticity_token, instead protect_from_forgery with null_session - CodeQL objected to a change it previously suggested and failed the build for not using.... - stop CodeQL complaining about update_attribute Remove reportable event tables from schema - they are accidental hangovers from working on a different spike branch, and should not be in this branch Rename reporting_app_redirect_url_with_auth_code_for to reporting_app_redirect_uri_with_auth_code_for as per Mishas suggestion stop the user getting caught in an infinite redirect loop when the reporting_app feature flag is disabled but they log in with a reporting app redirect param remove redundant :to_table use default keyword args remove redundant module-scoping remove redundant `and return` statements from client_id_error! use user: rather than user_id: as param for OneTimeToken.find_or_generate_for! combine migrations to add one_time_tokens and then add another column to it, into one migration better syntax for reliably updating reporting_app_session_token on logout, without CodeQL complaining about bypassing validations move the OneTimeToken model/controller under the Reporting:: scope move /tokens/authorize to /reporting-api/authorize use new /api/reporting routes for all reporting endpoints including authorization. Jira-Issue: MAV-1692 schema update after migrate Updates after rebasing onto next, which now incorporates the refactor of Organisations into Teams & sub-Teams. Some changes needed to the reporting auth interaction with the selected_org session variable, and test scenario setup Rename reporting_app_session_token to reporting_api_session_token Update scope of OneTimeToken after rebase
- For now only web service needs to recieve requests
- Use HTTP setup initially
- All communication in private subnet
- Communication in private subnet is further restricted via security
groups
- HTTPS requires a private certificate authority which comes with
significant price increase
- Also include new ECS service for reporting
- Since it is user-facing use CODE_DEPLOY deployment
- Needs a VALKEY instance for ensuring validity of login-tokens
Add necessary account-level resources
- Repository
- Deployment permission (ECS deployment only)
Modify variable and digest population for reporting service
- Ensure that the existing deployment workflows for web and goodjob
continue to function as normal
- Reporting service has a separate deployment workflow
- This will all be standardized once the IaC is extracted to its own
repository
Reference variable for port numbers
- Keeping it DRY
Include a shared secret for jwt signing
- Autorotated every Monday between 01:00-02:00 UTC
fix missing bracket (merge artefact?)
Create a service discovery setup for cross service communication
- For now only web service needs to recieve requests
- Use HTTP setup initially
- All communication in private subnet
- Communication in private subnet is further restricted via security
groups
- HTTPS requires a private certificate authority which comes with
significant price increase
- Also include new ECS service for reporting
- Since it is user-facing use CODE_DEPLOY deployment
- Needs a VALKEY instance for ensuring validity of login-tokens
Add necessary account-level resources
- Repository
- Deployment permission (ECS deployment only)
Modify variable and digest population for reporting service
- Ensure that the existing deployment workflows for web and goodjob
continue to function as normal
- Reporting service has a separate deployment workflow
- This will all be standardized once the IaC is extracted to its own
repository
Include a shared secret for jwt signing
- Autorotated every Monday between 01:00-02:00 UTC
Update deployment permissions
- also update healthcheck endpoint
update environment variables
…plement the new session cis2_info structure after the org -> team refactor remove redundant duplicated ensure_reporting_api_feature_enabled correct repo in adoc tidy up after rebase
aldavidson
force-pushed
the
spike/MAV-1406-auth-sharing-with-reporting
branch
from
ad27533 to
c76404f
Compare
|
|
2025-08-22 All meaningful code changes have now been merged into next as smaller PRs. There are only rebase artefacts and terraform changes left. The terraform was based on (and merged in from) the reporting_service_infrastructure branch - look there for the authoritative source of those TF changes, if they have not already been merged in also. We can now close this PR unmerged |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This spike is a proof-of-concept, to test the proposed approach for managing auth tokens between the Rails app and the Python reporting component
Jira-issue: MAV-1406
Note: this PR is now too big and complex to review in detail. We decided to convert it into several smaller, self-contained PRs which can be reviewed in isolation and therefore in more detail. Some of those smaller PRs have resulted in some further improvements. As a result, I'm going to change this PR back to
Draft, to avoid any risk of it being accidentally merged2025-08-22 All meaningful code changes have now been merged into
nextas smaller PRs. There are only rebase artefacts and terraform changes left. The terraform was based on (and merged in from) thereporting_service_infrastructurebranch - look there for the authoritative source of those TF changes, if they have not already been merged in also. We can now close this PR unmerged.