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

Parameterize client secret #347

Merged
merged 2 commits into from
Jul 11, 2018
Merged

Conversation

c-w
Copy link
Contributor

@c-w c-w commented Jul 10, 2018

This issue was found by @taupalosaurus. If a non-Google authentication provider is used, the client secret stored in the OKpy database will be different from this hard-coded value which causes the ok-client to ok-server communication to fail.

A work-around for this is to manually adjust the client secret in the ok-server database via the following command.

UPDATE client
SET client_secret='EWKtcCp5nICeYgVyCPypjs3aLORqQ3H'
WHERE client_id='ok-client';

However, a more long-term manageable fix is to parameterize the client secret and set it to the correct value for the authentication provider in each ok-client deployment.

If a non-Google authentication provider is used, the client secret
stored in the OKpy database will be different from this hard-coded
value which causes the ok-client to ok-server communication to fail.

A work-around for this is to manually adjust the client secret in the
ok-server database via the following command.

```sql
UPDATE client
SET client_secret='EWKtcCp5nICeYgVyCPypjs3aLORqQ3H'
WHERE client_id='ok-client';
```

However, a more long-term manageable fix is to parameterize the client
secret and set it to the correct value for the authentication provider
in each ok-client deployment.
@c-w c-w requested a review from Sumukh July 10, 2018 18:16
Copy link
Member

@colinschoen colinschoen left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@Sumukh Sumukh left a comment

Choose a reason for hiding this comment

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

Suggested a comment to make it clearer but looks good

CLIENT_SECRET = 'EWKtcCp5nICeYgVyCPypjs3aLORqQ3H'
# However, for other authentication providers such as Azure Active Directory
# this might not be the case
CLIENT_SECRET = os.getenv('OK_CLIENT_SECRET',
Copy link
Member

Choose a reason for hiding this comment

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

Could you make clear that this is the secret for the ok-client app on the ok service. I think that was unclear before. People can find the secret value at example.com/admin/clients/ok-client?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in d3a614a.

@c-w c-w merged commit 667013c into master Jul 11, 2018
@c-w c-w deleted the enhancement/c-w/parameterize-client-secret branch July 11, 2018 14:27
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.

3 participants