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

ORV2-2673 Rename Role to claim, auth group to role in vehicles and API alignment #1552

Merged
merged 21 commits into from
Aug 23, 2024

Conversation

krishnan-aot
Copy link
Collaborator

@krishnan-aot krishnan-aot commented Aug 22, 2024

Description

  • Rename roles to claims
  • Rename userAuthGroup to userRole.
  • Corresponding alignment changes in frontend to change userAuthGroup to userRole in request and response bodies.

Note that this is only a correction of the terminologies as we have so far been calling claims like READ-DOCUMENT as roles and user's roles like SYSADMIN, COMPANY_ADMIN as auth groups. This led to a lot of confusion around how to specify the permissions and conveying information to various stakeholders.

Some core terminology changes regarding permissions:

  • roles will be claims
  • userAuthGroup will be role

This is because, PPC_CLERK , SYS_ADMIN etc. among IDIR and COMPANY_ADMIN , PERMIT_APPLICANT among BCeID are all roles a user can take on. They should have been called roles to begin with.

READ-DOCUMENT, READ-PERMIT, WRITE-PERMIT etc. are all claims useful in a claim based authorization model. However, in our system, we never talk about these claims to business and for the most part, they aren't even aware of the development around this. They only care about the user's role like PPC_CLERK. These should have been called claims to begin with.

But because we misnamed claims as roles, we had to call the actual roles as userAuthGroups, we were stuck in a perennial confusion cycle of how to consolidate authorization and kept mixing up terminology and hence our permission implementation became a lot of custom calculation when in reality we needed a simple role based authorization scheme.

Hence the need for a correction.

Key Caveat:

  • While the entire backend is now free of active usage of the term USER_AUTH_GROUP, the DB and the reference to the DB column remains untouched since it involves breaking too many things and honestly not worth the effort at this stage. In future, it may be changed, however.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A
  • Test B

Checklist

  • I have read the CONTRIBUTING doc
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have already been accepted and merged

Further comments



Thanks for the PR!

Deployments, as required, will be available below:

Please create PRs in draft mode. Mark as ready to enable:

After merge, new images are promoted to:


Thanks for the PR!

Deployments, as required, will be available below:

Please create PRs in draft mode. Mark as ready to enable:

After merge, new images are promoted to:

Copy link
Collaborator

@praju-aot praju-aot left a comment

Choose a reason for hiding this comment

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

Approved Be changes

Copy link

sonarcloud bot commented Aug 23, 2024

Quality Gate Failed Quality Gate failed for 'onroutebc_policy'

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

Copy link

sonarcloud bot commented Aug 23, 2024

Quality Gate Failed Quality Gate failed for 'onroutebc dops'

Failed conditions
0.6% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

Copy link

sonarcloud bot commented Aug 23, 2024

Copy link

sonarcloud bot commented Aug 23, 2024

Quality Gate Failed Quality Gate failed for 'onroutebc vehicles'

Failed conditions
38.9% Coverage on New Code (required ≥ 80%)
5.1% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

Copy link

sonarcloud bot commented Aug 23, 2024

Quality Gate Failed Quality Gate failed for 'onroutebc frontend'

Failed conditions
46.6% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@krishnan-aot krishnan-aot merged commit 260e068 into main Aug 23, 2024
19 of 23 checks passed
@krishnan-aot krishnan-aot deleted the bep2 branch August 23, 2024 23:19
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.

3 participants