-
Notifications
You must be signed in to change notification settings - Fork 267
[User model] JWT token handling #1187
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
Conversation
This method takes a closure from the developer. The closure will be called when the Jwt token expires. The closure takes the externalId that is associated with the expired token as well as a completion block. The completion block should be called the app developer with the new jwt token for that user.
6379734 to
e4d4512
Compare
| var aliases: [String: String] = [:] | ||
|
|
||
| // TODO: We need to make this token secure | ||
| public var jwtBearerToken: String? |
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.
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.
Keychain normally isn't used for JWT. JWTs are designed to be short lived (normally 1 hour), so I don't think we need to do anything extra to secure it.
| internal extension OneSignalRequest { | ||
| func addJWTHeader(identityModel: OSIdentityModel) { | ||
| guard let token = identityModel.jwtBearerToken else { | ||
| return |
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.
May be good to have some message logged here
| } | ||
|
|
||
|
|
||
| private func fireJwtExpired() { |
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.
JWT jwt and Jwt are used I suggest removing Jwt and sticking with the other two
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.
@fhboswell there is jwt and Jwt being used as we are using camelcase. We could use JWT, but probably a bit easier to read as camelcase.
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.
@jkasten2
Jwt if fine if it is preferred however I think we should be consistent. We use JWT in OSUserRequest as part of this PR.
func addJWTHeader(identityModel: OSIdentityModel) {
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.
| } | ||
|
|
||
|
|
||
| private func fireJwtExpired() { |
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.
@fhboswell there is jwt and Jwt being used as we are using camelcase. We could use JWT, but probably a bit easier to read as camelcase.
| var aliases: [String: String] = [:] | ||
|
|
||
| // TODO: We need to make this token secure | ||
| public var jwtBearerToken: String? |
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.
Keychain normally isn't used for JWT. JWTs are designed to be short lived (normally 1 hour), so I don't think we need to do anything extra to secure it.
[User model] JWT token handling
[User model] JWT token handling
Description
One Line Summary
Using the JWT token for a user and adding the onJwtExpired method to User.
Details
Motivation
Enable JWT authorization for users
Scope
User module
Testing
Unit testing
N/A
Manual testing
Running on dev app
Affected code checklist
Checklist
Overview
Testing
Final pass
This change is