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

Meta token should contain client secret too #311

Closed
nivemaham opened this issue Aug 17, 2018 · 9 comments · Fixed by #310
Closed

Meta token should contain client secret too #311

nivemaham opened this issue Aug 17, 2018 · 9 comments · Fixed by #310
Assignees
Milestone

Comments

@nivemaham
Copy link
Collaborator

To allow configurable platform, the MP should also send the client secret with the meta-token. Otherwise, it will add a constraint of all platforms using the same secret. This has higher risk than sending secrets with meta-token.
FYI @blootsvoets, @dennyverbeeck , @yatharthranjan

@nivemaham nivemaham self-assigned this Aug 17, 2018
@nivemaham nivemaham added this to the 0.4.2 milestone Aug 17, 2018
@yatharthranjan
Copy link
Member

Yes this is very cool for a dynamic platform and app. But i only see one problem. The researchers sometimes have to send QR codes over the email either for remote recruitment or if the scanning did not work in person. In these cases, the QR code transfer is vulnerable and could compromise the client_secret. Right now, only refresh token is compromised which will expire and without client credentials no one can get access tokens. But if the client_secret is also included within it, this could pose a potential risk. Also since the meta token URL is exposed as no security is in place, this is a further risk.
What do you think ?

@nivemaham
Copy link
Collaborator Author

There is indeed a high risk involved IMO. However, from what I hear it is also easy to get the client credentials from Andriod app. Whoever installs the app can get this data it seems. If we do not send the secret, it will require all the platforms to use the same id and secret pairs. In that case, if a secret somehow gets exposed, it will potentially put the all of the platforms in risk.
Workarounds which comes to my mind are that we add restrictions around the solution.

  1. Avoid QR code transfer
    After all this implementation is aimed significantly reduce the issues with QR code scanning. Current QR code, literally has just one URL in it.
    image
    Old one is 1.51 KB
    image
    New one is 114 B.
    So we can assume that we will not transfer QR code by email.

  2. Fall-back when QR code scanning doesn't work with default configs..
    We have of deep link implementation in pipeline. My idea is that we can plan it as, when the QR-code scan doesn't work, we can show a text where the token id (12 character string) or the tokenUrl is visible. Preferably token id (better user experience). Currently, pRMT has default configs which are read from firebase configs. When the meta-QR code scanning is successful, it propagates an override of configurations. What we could indeed do with deep link is, ask the user to enter the token-id and let the pRMT fetch the token. However, this will only work for the platforms with default configs. This can give us a solution that balances between features and security.
    This is the scenario I have seen in many places where QR code scanning doesn't work.

@yatharthranjan
Copy link
Member

  1. For the first solution, it may not be entirely feasible. I agree it will generally reduce the transfer of QR codes but it may still need to be done in some cases. One that comes to mind is when a participants lives too far from the recruitment centre and is not willing to travel. In this case, a remote recruitment is performed where the QR code needs to be transferred.
  2. This solution seems the winner here. I have also seen an alternative way to login where the QR code does not work in many applications.

Another solution we can use is restrict the Android app to not display the client credentials. For example in pRMT, Right now i can access them in the android logs but I am assuming that we can hide these.

PS - The new QR code looks really nice. I can already see all the complexity gone.

@dennyverbeeck
Copy link
Contributor

I started typing, but here's the TLDR 😄: there is no issue with sharing the client secret for the mobile apps.

A bit of clarification regarding the client secret. The name 'secret' is misleading in these cases. Any mobile app or browser app is considered a public client, which means these apps can not keep a secret hidden. Therefore, for these clients, we only accept the authorization_code and, in some cases, the password grant types. This also means that any authorization is based on the logged in user, not on the client credentials, and therefore, sharing or even exposing the client secret really does not pose a problem.

In fact, the OAuth spec recommends that public clients like these use the authorization_code flow without even using the client secret (source).

Native applications that use the authorization code grant type SHOULD do so without using client credentials, due to the native application's inability to keep client credentials confidential.

Spring does not seem to support that mode of operation so that's why we still need it and have to keep using it , even though it's confusing to configure and maintain 'secrets' that are not secret and provide no security...

By contrast, e.g. the redcap integration app and RestAPI are applications that can keep their credentials a secret, since they are fully under our control. In cases like that, we can use the client_credentials grant type which means we trust the client based on the client id and secret provided, and authorize request based on that.

Hope this helps clear things up!

@yatharthranjan
Copy link
Member

I am a little confused now. I though we cannot use authorization_code grant type for apps since the subjects do not have a password. I thought it uses the refresh_token grant type.

Also, since Spring does not support the other types, if we add credentials to the QR codes, our system may be exposed since we are supplying both the refresh token and the credentials in a single QR code which has to be transferred via insecure means in certain cases. And even right now these credentials are exposed via public clients, so we are exposed anyway. Is that what you are saying @dennyverbeeck ? Is there no way of hiding these in the client apps ?

@blootsvoets
Copy link
Contributor

blootsvoets commented Aug 20, 2018

An attacker can easily get the secret of user clients by reading or modifying the Android APK or Javascript code, but there is a convenience barrier. In those cases, there is little protective value to the secret. The client_credentials flow should absolutely not be allowed user clients, only to server applications, because there, the secret is the only real protective measure.

@dennyverbeeck
Copy link
Contributor

Hi @yatharthranjan, we use the authorization_code grant type programatically on the backend to generate the first refresh token. It's true that 'normally', the participant would have to do this by logging in with a username and password. But early on in the project it was decided that we should aim to supply a means of pairing an app to a participant without the use of username/password, which is why we went for the QR code route. From a security point of view, sending a username and password to the subject, or sending them a QR code, is the same level of security. Both have an impact if intercepted. But the QR code is more convenient, and arguably more secure with the meta-token implementation, since it can only be used once.

The OAuth client credentials, which are the client id and client secret, are different than the username and password of the participant. They are only used to identify which OAuth client, i.e. which app, is acting on behalf of the user. E.g. the passive app is sending actigraphy data not by itself, but on behalf of the user. The passive app therefore has no authorities by itself, but through the use of the authorization_code flow, has the same authorities as the participant.

In contrast, an OAuth client which uses the client_credentials flow, does have authorities by itself, without a user being logged in. Which is why we only use that for applications where we can make sure the client secret can not be stolen, i.e. applications that run on our own servers.

In the meantime I've found out that we can configure an empty client secret in Spring, which does allow an OAuth client to authenticate without a client secret as well (see also this comment). Therefore I'm closing this and opening an issue to use an empty secret for public clients. @blootsvoets has made the appropriate issues in other projects here and here.

@yatharthranjan
Copy link
Member

Hi @dennyverbeeck, thanks for the explanation. It is much clear now to me.

@afolarin
Copy link
Member

afolarin commented Aug 29, 2018

@dennyverbeeck the empty credentials on the apps does sound better for managing the local clients credentials (which we can keep secret). We presumably need to notify the THINC-IT developer too.

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 a pull request may close this issue.

5 participants