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

Update device authorization and JWKS to closer match the spec #74

Merged
merged 4 commits into from
Jul 8, 2022

Conversation

mikenachbaur-okta
Copy link
Contributor

Copy link

@JayNewstrom JayNewstrom left a comment

Choose a reason for hiding this comment

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

I think the one missing thing from those 2 PRs is default scope.

https://github.com/okta/okta-mobile-swift/blob/master/Sources/AuthFoundation/Token%20Management/Token.swift#L168

I choose null, but there might be a more correct answer.

The rest looks good though.

@mikenachbaur-okta
Copy link
Contributor Author

Yeah, that seems like the right thing to do. Also, Okta has a feature for defining a default scope for when a client doesn't supply one, so we might want to make the scope argument optional for our authorization flows.

let data = data(for: """
{
"token_type": "Bearer",
"expires_in": 3600,

Choose a reason for hiding this comment

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

Worth noting, some Authorization servers may not return expires_in (notably when you are doing pure OAuth, without OIDC). I don't think it's something we need to account for, but I think the only 2 things that are required from OAuth are access_token/token_type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'm not sure if we should worry about that. There's a lot of code that assumes the expires_in exists, for both Swift and Kotlin SDKs. So I'd hold off on this, until it's necessary.

@mikenachbaur-okta mikenachbaur-okta merged commit f494897 into master Jul 8, 2022
@mikenachbaur-okta mikenachbaur-okta deleted the man-OKTA-512299-UpdateSpec branch July 8, 2022 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants