Skip to content

feat(api): Add organization_id_or_slug Support to Customer Domain Middleware#70172

Merged
iamrajjoshi merged 2 commits into
masterfrom
raj/feat/idorslug/customer-domain
May 6, 2024
Merged

feat(api): Add organization_id_or_slug Support to Customer Domain Middleware#70172
iamrajjoshi merged 2 commits into
masterfrom
raj/feat/idorslug/customer-domain

Conversation

@iamrajjoshi

@iamrajjoshi iamrajjoshi commented May 2, 2024

Copy link
Copy Markdown
Collaborator

We are updating our APIs work with id and slug path params. We need to make sure that the middleware works with both type of path params. We need to make sure that if an orgid is passed, we don't redirect them.

For more context: https://sentry.slack.com/archives/C032E82D338/p1714605291155389

Example: acme.sentry.io/organizations/12345/issues to http://12345.sentry.io/issues.

@iamrajjoshi iamrajjoshi requested review from a team and markstory May 2, 2024 19:53
@iamrajjoshi iamrajjoshi self-assigned this May 2, 2024
@iamrajjoshi iamrajjoshi requested a review from a team as a code owner May 2, 2024 19:53
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 2, 2024
@iamrajjoshi iamrajjoshi requested review from a team and dashed May 2, 2024 20:02

@sentaur-athena sentaur-athena left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Changes makes sense to be but a hybrid cloud stamp would be nice. @dashed ?

result.kwargs
and "organization_slug" in result.kwargs
and result.kwargs["organization_slug"] != activeorg
org_slug_path_mismatch = result.kwargs and (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

To make this easier to understand, with this solution, if ID is passed in organization_id_or_slug and it is not the correct ID of the active org redirect_url stays activeorg url but kwargs["organization_id_or_slug"] remain the wrong ID so this leads to 404. I think it's expected but just clarifying.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yep thats correct. In general, this scenario shouldn't be happening because the org_id should only be used in subdomain-less queries.

Comment on lines +415 to +421
# No redirect for id
response = self.client.get(
reverse("org-events-endpoint-id-or-slug", kwargs={"organization_id_or_slug": 1234}),
data={"querystring": "value"},
HTTP_HOST="albertos-apples.testserver",
follow=True,
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In this scenario in saas, we'll end up going through the API gateway middleware and being proxied to the organization's region based on the 1234 slug and the subdomain will be ignored. I think that is a reasonable outcome as all slug subdomains resolve to control silo instances.

@iamrajjoshi iamrajjoshi merged commit 902b81c into master May 6, 2024
@iamrajjoshi iamrajjoshi deleted the raj/feat/idorslug/customer-domain branch May 6, 2024 16:14
@github-actions github-actions Bot locked and limited conversation to collaborators May 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants