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

Remove button for re-generating access token for API user #2620

Merged

Conversation

floehopper
Copy link
Contributor

@floehopper floehopper commented Jan 3, 2024

The "Re-generate" button revokes the access token and creates a new one for the same API user and application. However, in the meantime application(s) will continue to use the old (now revoked) access token and thus API requests will fail. A separate step is needed to sync the new valid access token to the relevant application container(s).

Screenshot 2024-01-03 at 11 37 02

I suspect this is why the documentation used by #govuk-2ndline-tech recommends first creating a new access token and only revoking the old one once the new one has been synced to the relevant application container(s) and is confirmed to be working OK.

I've also looked at the count of EventLog records in production which seems to confirm that the "Re-generate" button has rarely been used:

  • EventLog::ACCESS_TOKEN_GENERATED: 88
  • EventLog::ACCESS_TOKEN_REVOKED: 74
  • EventLog::ACCESS_TOKEN_REGENERATED: 4

I asked about this in Slack and @theseanything confirmed that the button would not be missed, because it's still possible to achieve the same effect via the UI albeit with more clicks.

Ideally we'd be able to trigger the syncing of access tokens from within the Signon codebase and fully automate the procedure that 2nd Line use. However, that's a bigger piece of work and out-of-scope for the moment.

I'm about to move the "Manage tokens for API user" page to use the GOV.UK Design System. Doing this first will make that easier.

I've left the EventLog::ACCESS_TOKEN_REGENERATED constant defined in order to support the historical records. However, I've changed an unrelated test to use a different constant and added a comment to make it clear that this constant is deprecated.

Copy link
Contributor

@chrisroos chrisroos left a comment

Choose a reason for hiding this comment

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

Looks good to me, @floehopper 👍

The "Re-generate" button revokes the access token and creates a new one
for the same API user and application. However, in the meantime
application(s) will continue to use the old (now revoked) access token
and thus API requests will fail. A separate step is needed to sync the
new valid access token to the relevant application container(s).

I suspect this is why the documentation [1] used by #govuk-2ndline-tech
recommends first creating a new access token and only revoking the old
one once the new one has been synced to the relevant application
container(s) and is confirmed to be working OK.

I've also looked at the count of `EventLog` records in production which
seems to confirm that the "Re-generate" button has rarely been used:
* `EventLog::ACCESS_TOKEN_GENERATED`: 88
* `EventLog::ACCESS_TOKEN_REVOKED`: 74
* `EventLog::ACCESS_TOKEN_REGENERATED`: 4

I asked about this in Slack and @theseanything confirmed that the button
would not be missed, because it's still possible to achieve the same
effect via the UI albeit with more clicks.

I'm about to move the "Manage tokens for API user" page to use the
GOV.UK Design System [2]. Doing this first will make that easier.

I've left the `EventLog::ACCESS_TOKEN_REGENERATED` constant defined in
order to support the historical records. However, I've changed an
unrelated test to use a different constant and added a comment to make
it clear that this constant is deprecated.

[1]: https://docs.publishing.service.gov.uk/manual/alerts/signon-api-user-token-expires-soon.html
[2]: https://trello.com/c/75Jyg8zR
@floehopper floehopper force-pushed the remove-regenerate-access-token-button-for-api-user branch from 785608f to 32ac507 Compare January 3, 2024 14:21
@floehopper floehopper merged commit 231c128 into main Jan 3, 2024
16 checks passed
@floehopper floehopper deleted the remove-regenerate-access-token-button-for-api-user branch January 3, 2024 14:25
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