Skip to content

feat: add ulims authz policy#310

Merged
MattPrit merged 2 commits into
mainfrom
feat/mp/add-ulims
Jun 23, 2026
Merged

feat: add ulims authz policy#310
MattPrit merged 2 commits into
mainfrom
feat/mp/add-ulims

Conversation

@MattPrit

@MattPrit MattPrit commented May 26, 2026

Copy link
Copy Markdown
Collaborator

Summary: Adds authZ policy for use in ULIMS services. Essentially, this moves the existing ULIMS policy into the central repository, with the addition of service account support and a new rule for filtering a list of instruments

Added rules:

  • allow - top-level check that the user has been verified
  • session_restrictions - returns the sessions that the user has access to, in the format [{"beamline": "i03", "proposal_number": 1, "visit_number": 1}, ...]
  • filter_sessions - given an input instrument_sessions of (proposal number, session number) pairs, return those pairs corresponding to session that the user has access to
  • filter_instruments - given an in put instruments, a list of instrument names, return those that the user has access to (i.e is an admin of)

Note: There is a section of policy that I have copied from tiled.rego

Note: Some of the rules depend on token.claims.beamline, I suspect that more generally we would like token.claims.instruments, but I have used beamline for consistency with tiled.rego

@MattPrit MattPrit marked this pull request as ready for review May 27, 2026 10:40
@MattPrit MattPrit force-pushed the feat/mp/add-ulims branch from eb36e68 to 6e60748 Compare May 27, 2026 10:41
@MattPrit MattPrit requested a review from ZohebShaikh May 27, 2026 11:02

@ZohebShaikh ZohebShaikh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I like the docs that you have put for the policy, We should add a regal lint for this if possible in a separate PR.

Note: Some of the rules depend on token.claims.beamline, I suspect that more generally we would like token.claims.instruments, but I have used beamline for consistency with tiled.rego

I agree we should make the change from beamline to instrument. The faster we do this the better as it will have lesser impact. This needs to be changed from the bundler as well

Comment thread policy/diamond/policy/ulims/ulims.rego Outdated
Comment thread policy/diamond/policy/ulims/ulims.rego
Comment thread policy/diamond/policy/ulims/ulims.rego Outdated
Comment thread policy/diamond/policy/ulims/ulims.rego Outdated
Comment thread policy/diamond/policy/ulims/ulims.rego Outdated
Comment thread policy/diamond/policy/ulims/ulims.rego Outdated
Comment thread policy/diamond/policy/ulims/ulims.rego Outdated
Comment thread policy/diamond/policy/ulims/ulims.rego Outdated
@MattPrit

MattPrit commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator Author

I agree we should make the change from beamline to instrument. The faster we do this the better as it will have lesser impact...

What about multi-valued claims (i.e. multiple instrument access)? Should I move to token.claims.instrument and assume a single value for now?

@ZohebShaikh

ZohebShaikh commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

What about multi-valued claims (i.e. multiple instrument access)? Should I move to token.claims.instrument and assume a single value for now?

I'm little reluctant to add instruments because I feel like the UDC client should be 1 per beamline and I have not come across a use case where 1 beamline needs access to 2 or more beamlines data.
So I think we should leave it to a single value for now

@MattPrit MattPrit requested a review from ZohebShaikh June 4, 2026 12:37
ZohebShaikh
ZohebShaikh previously approved these changes Jun 23, 2026
@ZohebShaikh

Copy link
Copy Markdown
Collaborator

Apologies I was on holiday then on sick leave and this might have slipped through the cracks. Looks Good
Only thing to do before merge is to squash/ rewrite commit history

@MattPrit MattPrit force-pushed the feat/mp/add-ulims branch from fd6feeb to 6517994 Compare June 23, 2026 12:25
@MattPrit MattPrit merged commit a287d87 into main Jun 23, 2026
36 checks passed
@MattPrit MattPrit deleted the feat/mp/add-ulims branch June 23, 2026 13:22
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