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(backend): tenant support for wallet address (#3114) #3152

Draft
wants to merge 33 commits into
base: 2893/multi-tenancy-v1
Choose a base branch
from

Conversation

koekiebox
Copy link
Collaborator

@koekiebox koekiebox commented Dec 5, 2024

Changes proposed in this pull request

Context

Checklist

  • Related issues linked using fixes #number
  • Tests added/updated
  • Make sure that all checks pass
  • Bruno collection updated (if necessary)
  • Documentation issue created with user-docs label (if necessary)
  • OpenAPI specs updated (if necessary)

@koekiebox koekiebox added the do not merge Do not merge PRs with these label label Dec 5, 2024
@koekiebox koekiebox self-assigned this Dec 5, 2024
@github-actions github-actions bot added type: tests Testing related pkg: backend Changes in the backend package. pkg: frontend Changes in the frontend package. type: source Changes business logic pkg: mock-ase pkg: mock-account-service-lib labels Dec 5, 2024
@koekiebox koekiebox changed the title 3114/tenanted wallet addresses feat(backend): tenant support for wallet address (#3114) Dec 5, 2024
@koekiebox koekiebox changed the base branch from 2893/multi-tenancy-v1 to nl/3123/backend-tenant-service December 5, 2024 13:25
@github-actions github-actions bot added the pkg: auth Changes in the GNAP auth package. label Dec 9, 2024
@@ -1213,6 +1226,8 @@ type CreateReceiverResponse {
}

input CreateWalletAddressInput {
"Unique identifier of the tenant associated with the wallet address. This cannot be changed."
tenantId: String!
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This would need to be removed at a later stage (once we obtain the tenant from the signature):
#2928

Base automatically changed from nl/3123/backend-tenant-service to 2893/multi-tenancy-v1 December 9, 2024 17:32
@mkurapov mkurapov linked an issue Dec 9, 2024 that may be closed by this pull request
4 tasks
@@ -53,6 +62,9 @@ export class WalletAddress
public readonly assetId!: string
public asset!: Asset

public readonly tenantId!: string
public tenant!: Tenant
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we actually need the tenant!: Tenant property on the model? I'm not sure we'd end up using it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Originally I thought we would need it, but since we have authServer and resourceServer, we are good. Will update.

@@ -176,13 +181,19 @@ async function createWalletAddress(
const walletAddress = await WalletAddress.query(
deps.knex
).insertGraphAndFetch({
tenantId: options.tenantId,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we would need the gets and update to also take in tenantId in the query, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think so, since you cannot move a wallet between tenants? Unless that is something we want? @mkurapov

@github-actions github-actions bot removed the pkg: auth Changes in the GNAP auth package. label Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge Do not merge PRs with these label pkg: backend Changes in the backend package. pkg: frontend Changes in the frontend package. pkg: mock-account-service-lib pkg: mock-ase type: source Changes business logic type: tests Testing related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Multi-Tenant] Tenanted Wallet Addresses
3 participants