Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

AGDROID-686 - Add Token Verification [Do No Merge] #12

Closed
wants to merge 3 commits into from

Conversation

tomjackman
Copy link
Contributor

@tomjackman tomjackman commented Jan 19, 2018

Description
[Do Not Merge]
Function to Verify JWT Tokens (Access, Identity, Refresh) using a Keycloak Realm Public Key before they are used as part of client side access control decisions.

Tested successfully with both valid & tampered Access/Identity tokens. Identity Brokering originated tokens can also be validated.

To Do

  • Get the Public Key from the Mobile Core Config (when available)
  • Get the Audience from the Mobile Core Config (when available)
  • Construct the Issuer using properties in the Mobile Core Config (when available)
  • Add Unit tests - Test Wrong issues, audiences, tampered signature, payload, algorithm info sections etc

@wei-lee
Copy link
Contributor

wei-lee commented Jan 19, 2018

For maximum security, I think it's best to get the public key of the server from the backend during the validation. We can not trust the public key that is returned in Mobile Core config

@tomjackman
Copy link
Contributor Author

tomjackman commented Jan 19, 2018

If someone tampers with the public key in the mobile core config alone, this will fail the verification checks straight up as we would hope anway. The only way they could possibly get around verification is if they they edit both the mobile core config and the authState, wherever it is stored. Ideally the authState should be stored in the keystore, or encrypted with a key that is stored in the keystore.

If we do online checks when online, having to send a network request for each time you want to click on a menu button, or render a view that has a number of role checks to show/hide certain content in a view sounds like it could cause cause a poor user experience if there is a lag or delay between tapping on various menu items, particularly in areas with poor network connection.

If we can't trust any data in the mobile core config, then its being stored in the wrong place, given as this will contain instructions or inputs as to how the app will behave, it shouldn't be in some plaintext file.

I had a look at openid/AppAuth-Android#203 but this is for the identity token, rather than the access token, and it doesn't support offline verification.

@wei-lee
Copy link
Contributor

wei-lee commented Jan 19, 2018

@TommyJ1994 yes, I think you are right. It's more important to secure the authState data so that it can not be tampered with, and then we can perform the verification locally. It is also a lot simpler to not have to fetch the public key from the backend server.

So I agree for now let's keep the public key in the config file. Let's add the ticket in the backlog to encrypt the authState file locally to improve security.


// TODO Get the Public Key from the Mobile Core Config
// TODO Get the Audience from the Mobile Core Config
// TODO Construct the Issuer using properties in the Mobile Core Config
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aidenkeating So the audience will just be the clientID thats in the mobile core config.

The issuer will look like the following in the decoded JWT "iss": "https://keycloak.security.feedhenry.org/auth/realms/client-app" so this will need to be constructed with something like AUTH_SERVER_URL + "/realms/" + CLIENT_ID. These can be gotten from the mobile core config too.

I think we can leave the public key as a parameter for now actually. In terms of the format, it will just be a public key without the BEGIN/END tags, as this is how keycloak displays it in the portal under. Realm Settings (Menu Item) > Keys (Tab Item) > Public Key (Button). The public key should show in a popup then.

I think the config might be defined in this class, and this class will get the config from the mobile core config then - https://github.com/aerogear/aerogear-android-sdk/blob/33f42d7718e7abf06604ce888bdeb3f0ee2f4001/auth/src/main/java/org/aerogear/auth/AuthServiceConfig.java

Copy link
Contributor

Choose a reason for hiding this comment

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

@TommyJ1994 Do we expect the user of the SDK to be invoking this function? From the original issue in Jira I'm assuming this should be hidden from the user? (https://issues.jboss.org/browse/AGDROID-686)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aidenkeating Originally, I think it would be of benefit for this to be called under the hood as part of the get roles functions, but its probably not guaranteed that a developer will put/have the public key in the mobile core config.

I was thinking we could maybe just expose them. We could probably do some method overloading so there are two functions:

private boolean verifyToken(String token) {
// get public key from the mobile core config
}

private boolean verifyToken(String token, String publicKey, String audience, String issuer) {
// get values from the from the params
}

This will allow an end user to specify the expected JWT info, rather than the one from the mobile core config.

@wei-lee
Copy link
Contributor

wei-lee commented Feb 7, 2018

@TommyJ1994 is this PR replaced by #21

@tomjackman
Copy link
Contributor Author

@wei-lee Yes, I had to jump off this last week. Closing now.

@tomjackman tomjackman closed this Feb 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants