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(users): add tenant id reads in user roles #6661

Merged
merged 8 commits into from
Nov 29, 2024

Conversation

apoorvdixit88
Copy link
Contributor

@apoorvdixit88 apoorvdixit88 commented Nov 26, 2024

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

Support tenant id in user roles queries.

Additional Changes

  • This PR modifies the API contract
  • This PR modifies the database schema
  • This PR modifies application configuration/environment variables

Motivation and Context

Closes #6660

How did you test it?

With tenancy feature flag enabled these changes will be tested when upcoming tenant related PRs from dashboard gets merged.

The current behaviour of user apis should not change. Since hyperswitch has its own tenant id and we don't have tenant_id feature flag enabled for now.
Tested the sanity flows for users. Working as expected.

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed the submitted code
  • I added unit tests for my changes where possible

@apoorvdixit88 apoorvdixit88 added C-feature Category: Feature request or enhancement S-waiting-on-review Status: This PR has been implemented and needs to be reviewed A-users Area: Users labels Nov 26, 2024
@apoorvdixit88 apoorvdixit88 self-assigned this Nov 26, 2024
@apoorvdixit88 apoorvdixit88 requested review from a team as code owners November 26, 2024 09:27
Copy link

semanticdiff-com bot commented Nov 26, 2024

Review changes with  SemanticDiff

Changed Files
File Status
  crates/router/src/types/domain/user/decision_manager.rs  59% smaller
  crates/router/src/db/user_role.rs  51% smaller
  crates/diesel_models/src/query/user_role.rs  16% smaller
  crates/router/src/analytics.rs  0% smaller
  crates/router/src/core/user.rs  0% smaller
  crates/router/src/core/user_role.rs  0% smaller
  crates/router/src/db/kafka_store.rs  0% smaller
  crates/router/src/services/authentication.rs  0% smaller
  crates/router/src/utils/user_role.rs  0% smaller

crates/diesel_models/src/query/user_role.rs Outdated Show resolved Hide resolved
crates/diesel_models/src/query/user_role.rs Outdated Show resolved Hide resolved
crates/router/src/types/domain/user/decision_manager.rs Outdated Show resolved Hide resolved
crates/router/src/types/domain/user/decision_manager.rs Outdated Show resolved Hide resolved
crates/router/src/utils/user_role.rs Outdated Show resolved Hide resolved
@@ -1864,6 +1864,7 @@ pub mod routes {
.global_store
.list_user_roles_by_user_id(ListUserRolesByUserIdPayload {
user_id: &auth.user_id,
tenant_id: auth.tenant_id.as_ref().unwrap_or(&state.tenant.tenant_id),
Copy link
Contributor

Choose a reason for hiding this comment

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

If default value if taken for tenant id not being present in auth token, then won't a user be able to access any tenancy if user gets access to a token with tenant_id: None.
Are we ensuring tenant_id will never be null ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tenant_id will always be present in token, i kept it option just for backward compatibility. We will be making it non option after one two deployments.
Also token tenant's id and state tenant id should be same, so we can use any. We are checking for this only in authentication part.
Moreover currently the feature is disabled, so we won't be having different tenancy support.

ThisIsMani
ThisIsMani previously approved these changes Nov 26, 2024
tsdk02
tsdk02 previously approved these changes Nov 26, 2024
dracarys18
dracarys18 previously approved these changes Nov 27, 2024
tsdk02
tsdk02 previously approved these changes Nov 27, 2024
@apoorvdixit88 apoorvdixit88 dismissed stale reviews from tsdk02 and dracarys18 via 998f369 November 28, 2024 08:11
@Gnanasundari24 Gnanasundari24 added this pull request to the merge queue Nov 29, 2024
Merged via the queue into main with commit 9212f77 Nov 29, 2024
15 of 17 checks passed
@Gnanasundari24 Gnanasundari24 deleted the add-tenant-id-in-user-roles-queries branch November 29, 2024 10:54
@SanchithHegde SanchithHegde removed the S-waiting-on-review Status: This PR has been implemented and needs to be reviewed label Dec 1, 2024
pixincreate added a commit that referenced this pull request Dec 2, 2024
…ed-cypress

* 'main' of github.com:juspay/hyperswitch:
  chore(version): 2024.12.02.0
  refactor(users): Use domain email type in user DB functions (#6699)
  fix(openapi): Standardise API naming scheme for V2 (#6510)
  feat(users): add tenant id reads in user roles (#6661)
  fix(users): Mark user as verified if user logins from SSO (#6694)
  feat(payments): [Payment links] add showCardFormByDefault config for payment links (#6663)
  feat(connector): [REDSYS] add Connector Template Code (#6659)
  refactor(currency_conversion): release redis lock if api call fails (#6671)
  feat(connector): [Adyen] Fetch email from customer email for payment request (#6676)
  fix(router): populate card network in the network transaction id based MIT flow (#6690)
  refactor(router): [ZSL] remove partially capture status (#6689)
  fix(opensearch): handle empty free-text query search in global search (#6685)
  fix(analytics): fix first_attempt filter value parsing for Payments (#6667)
  chore(version): 2024.11.29.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-users Area: Users C-feature Category: Feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(user_roles): support tenant_id reads
7 participants