-
Notifications
You must be signed in to change notification settings - Fork 9
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: refactored cloud auth #317
Conversation
Show Plan
Action: |
@@ -149,6 +140,9 @@ pub async fn handler( | |||
return Err(e); | |||
} | |||
|
|||
// Ensure tenant real | |||
let _existing_tenant = state.tenant_store.get_tenant(&id).await?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving after, auth comes first
Show Plan
Action: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one! One thing that we could look into in a separate PR is to using staging registry for project id validation, like Notify Server does.
Description
Uses the new Cloud authentication mechanism introduced in this PR. Slack conversation
Instead of a Supabase user JWT being provided to Push Server, Push Server authenticating this JWT with a shared secret from Cloud, and then Push Server checking with Push Server if the user has access to the tenant... now the authorization of the user's access to the project/tenant is completely offloaded to Cloud and all Push Server does now is checks a JWT signature. This JWT is now signed by Cloud app with a different shared secret and has a TTL of 30s rather than being the same one used for all user authentication in Cloud.
Resolves that Cloud currently is not able to configure Push Server due to a breaking change that was overlooked.
Remaining work:
JWT_SECRET
to new secretCLOUD_API_SECRET
secretHow Has This Been Tested?
Not tested
Due Diligence