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

feat: UI changes for Authz caching #88

Merged
merged 6 commits into from
Jun 13, 2023
Merged

feat: UI changes for Authz caching #88

merged 6 commits into from
Jun 13, 2023

Conversation

krithika369
Copy link
Collaborator

@krithika369 krithika369 commented Jun 11, 2023

This MR mainly adds a small UI change to warn the users that, when updating permissions for a project, changes may take up to X minutes to take effect in the individual components. Since the individual components may set any value for KeyExpirySeconds, a new MaxKeyExpirySeconds has been introduced in api/pkg/authz/enforcer/enforcer.go which is hard-coded to 10 minutes. This means that, components will not be allowed to set key expiry > this value (this will be the X).

Illustration

Screenshot 2023-06-11 at 7 38 53 PM

Changes

  • api/cmd/main.go - Supply UI env var REACT_APP_MAX_AUTHZ_CACHE_EXPIRY_MINUTES. When doing this, whether Authz and caching are enabled on the MLP app does not matter because they could be turned off on MLP but turned on in Turing, Merlin, etc.
  • api/pkg/authz/enforcer/enforcer.go - Add MaxKeyExpirySeconds and a check to ensure that, when initialising the enforcer, the configured KeyExpirySeconds cannot be larger.
  • ui/packages/app/src/config.js - Add MAX_AUTHZ_CACHE_EXPIRY_MINUTES config
  • ui/packages/app/src/project_setting/user_role/SubmitUserRoleForm.js - Add info in the edit user permissions form

@krithika369 krithika369 marked this pull request as draft June 11, 2023 11:54
@krithika369 krithika369 temporarily deployed to manual June 11, 2023 23:49 — with GitHub Actions Inactive
@krithika369 krithika369 marked this pull request as ready for review June 11, 2023 23:51
Copy link
Contributor

@deadlycoconuts deadlycoconuts left a comment

Choose a reason for hiding this comment

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

The changes LGTM but just to understand the intention of the changes further, REACT_APP_MAX_AUTHZ_CACHE_EXPIRY_MINUTES is expected to be 'fixed' as the default value 600 / 60 minutes right? i.e. we aren't expecting anyone to be manually toggling that UI env var, are we?

@krithika369
Copy link
Collaborator Author

The changes LGTM but just to understand the intention of the changes further, REACT_APP_MAX_AUTHZ_CACHE_EXPIRY_MINUTES is expected to be 'fixed' as the default value 600 / 60 minutes right? i.e. we aren't expecting anyone to be manually toggling that UI env var, are we?

Correct. However the env var is still useful when the UI is not being served by the API (as with local dev).

@krithika369
Copy link
Collaborator Author

Thank for the quick review, @deadlycoconuts ! Will be merging this shortly.

@krithika369 krithika369 merged commit dd63f43 into main Jun 13, 2023
@krithika369 krithika369 deleted the add_keto_cache_ui branch June 13, 2023 01:09
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