RW-12581 Remove user enabled check when redundant #4534
Conversation
| authProvider.checkUserEnabled( | ||
| userInfo | ||
| ) // when request doesn't have cloudContext defined, we check if user is enabled | ||
| } |
There was a problem hiding this comment.
Moving up the check if cloudContext is not defined; otherwise, remove this redundant check
| val readerGoogleProjectIds: Set[ProjectSamResourceId], | ||
| val readerRuntimeIds: Set[SamResourceId], | ||
| val readerWorkspaceIds: Set[WorkspaceResourceSamResourceId] | ||
| final case class AuthorizedIds(ownerGoogleProjectIds: Set[ProjectSamResourceId], |
There was a problem hiding this comment.
these vals are redundant
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #4534 +/- ##
===========================================
- Coverage 73.65% 73.64% -0.02%
===========================================
Files 158 158
Lines 14699 14700 +1
Branches 1160 1119 -41
===========================================
- Hits 10827 10826 -1
- Misses 3872 3874 +2
Continue to review full report in Codecov by Sentry.
|
|
Change LG. But I throught all Terra services use the same proxy config, and the proxy will call SAM to check user ToS and enable status already. Why we need this change in Leo again ? |
|
@yonghaoy that's true... @mlilynolting do you know why we added the explicit user enabled call in the first place? |
jenkins retest |
|
@Qi77Qi just FYI. Our integration tests do not run on jenkins anymore. You can retrigger via GHA instead |
When
cloudContextis defined, we're already checking if a user has project reader permission, which also makes sure the user is active, so we don't need to check whether user is enabled in these cases.Jira ticket: https://broadworkbench.atlassian.net/browse/[ticket_number]
Summary of changes
What
Why
Testing these changes
What to test
Who tested and where
jenkins retestorjenkins multi-test.