-
Notifications
You must be signed in to change notification settings - Fork 598
First pass on documentation for auth claims #3218
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
base: master
Are you sure you want to change the base?
Conversation
d111139
to
c5b0ff8
Compare
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.
I took a look at the rust examples, and my main thoughts are that:
- We should never use
unwrap
, and - We should probably have some example of a reducer other than the
ClientConnected
one.
```rust | ||
#[reducer(client_connected)] | ||
pub fn connect(ctx: &ReducerContext) -> Result<(), String> { | ||
let auth_ctx = ctx.sender_auth(); |
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.
It's generally better to avoid using unwrap, so it's more obvious that it can't panic.
let auth_ctx = ctx.sender_auth(); | |
let jwt = ctx.sender_auth() | |
.jwt() | |
.ok_or_else(|| "Client connected without JWT".to_string())?; |
If you think having those branches is more readable, you can also use a match:
let auth_ctx = ctx.sender_auth(); | |
let jwt = match ctx.sender_auth().jwt() { | |
Some(jwt) => jwt, | |
None => return Err("Client connected without JWT".to_string()), | |
}; | |
let jwt = match ctx.sender_auth() | |
.jwt() | |
.ok_or_else(|| "Client connected without JWT".to_string())?; |
let claims: CustomClaims = serde_json::from_slice(payload.as_bytes()).map_err(|e| format!("Client connected with invalid JWT: {}", e).to_string())?; | ||
|
||
// In this example you would really want to check the issuer of the JWT to ensure it is from a trusted provider | ||
if auth_ctx.jwt().unwrap().issuer() != "https://accounts.google.com" { |
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.
You could use claims.iss
here instead of auth_ctx.jwt().unwrap().issuer()
.
pub struct User { | ||
#[primary_key] | ||
#[auto_inc] | ||
pub id: u64, |
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.
Using these auto_inc
user ids in examples feels like it clashes with the identity
that we are using in so many parts of our APIs.
|
||
## Example: automatically give users a role based on their auth claims | ||
|
||
When users first connect to your SpacetimeDB module you may want to automatically give them some role. For example, at clockwork we might want to give users with a valid "clockworklabs.io" email an admin role. We can do this by checking the email claim in the connect reducer and giving them the admin role if they have a valid email. |
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.
I like the example of roles. I think it could use useful to add an example of using this in a reducer to decide whether a client is allowed to do something, since these examples are only covering the initial connection right now.
} | ||
|
||
#[table(name = user_role)] | ||
pub struct UserRole { |
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.
I'm not sure how useful these tables would be, since the reducers that would be enforcing any security rules would also have access to the email on the auth token. Having a table with email
and role
would make more sense to me.
Since the code in this example is computing the role from the email, we don't actually need to store any of the roles in tables. I think the main reason to have a user roles table would be to be able to set roles for different users via SQL or a different admin-only reducer.
|
||
let jwt = auth_ctx.jwt().unwrap(); | ||
// Example: We only accept google auth | ||
if jwt.issuer() != "https://accounts.google.com" { |
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.
One weird aspect of this is that people using maincloud have auth from auth.spacetimedb.com
, so this would mean that someone logged in with the creds needed to publish would not be able to use spacetime sql
.
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.
#[table(name = admin)]
pub struct Admin {
#[unique]
pub ident: Identity,
}
// Add to on_connect
#[reducer(init)]
pub fn init(ctx: &ReducerContext) -> Result<(), String> {
ctx.db.admin().insert(Admin{
ident: ctx.sender
});
Ok(())
}
// Is the sender an owner of the db?
if ctx.db.admin().ident().find(ctx.sender).is_some() {
return Ok(());
}
// Then check for google auth
// Note: check the audience here as well
Description of Changes
This PR adds documentation for accessing auth claims in modules.
API and ABI breaking changes
This is a docs change.
Expected complexity level and risk
0
Testing