-
Notifications
You must be signed in to change notification settings - Fork 0
Add service token exchange method #6
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
|
The latest Buf updates on your PR. Results from workflow Validate Pull Request / check_buf (pull_request).
|
|
(You have to bump the version in package.json lest Actions be sad) So the idea is that we would move to Looks like that method somehow escaped having TokenInfo looks good. I noticed there's a title_id field in the current tokens, is that worth doing anything with? |
I know, I intentionally left it not updated since there's also an existing PR and we don't know the final version number right now. Same reason why I haven't pushed any generated code, because we aren't sure of the final protobufs
Correct 👍 I figured we'd want the same data, so I just reused the return type, but if we want to edit it here we can (though I don't see why we would)
Got it, nice catch 👍
Probably not? Ideally service should know its own title ID(s). It can be added though (I could just yeet back the entire token struct tbh) |
Oh, I believe the Actions do that for you anyway. Yeah, no, I want the same user document for both OAuth and service token flows, it makes the TypeScript much easier to wrangle. Currently we're using
Maybe there's some validation reason? Though yeah it feels like a stretch |
|
That should cover it all I think? Went ahead and bumped the version and marked as ready for review now |
ashquarky
left a comment
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.
Sorry I forgor about this. API LGTM
Resolves #XXX
Changes:
As per PretendoNetwork/account#189 (comment), the account server no longer has a way to validate service tokens. This is intended, as service tokens are special and need to be handled special
These changes adds a new
ExchangeServiceTokenForUserDatamethod to theAccountservice. I am only focused on our needs right now, which is why theAPIservice has not been touched (as that service is supposed to be a replacement for the "public" http-based API used by the website, theAccountservice is for our internal needs)Notably it separates token and user info into their own messages, so that services can be told of when the token was issued so they can handle expiry as they see fit
Marked as a draft to allow for discussion about the needs of the returned data. Right now the same user data as returned by
ExchangeTokenForUserDatais used, but we can change thisThe
client_idfield is the same ID used to get a service token from NNAS. This is used so that NNAS can lookup that services encryption key. For example, Juxt would send87cd32617f1985439ea608c2746e4610(the Olive client ID)CC @ashquarky. Once this is merged, and the account server updated, Juxt should never have to worry about decrypting tokens again