introduces per-user API tokens via 'apikey: true'#30
Open
mattoberle wants to merge 2 commits intomasterfrom
Open
introduces per-user API tokens via 'apikey: true'#30mattoberle wants to merge 2 commits intomasterfrom
mattoberle wants to merge 2 commits intomasterfrom
Conversation
This commit introduces a rudimentary approach to assigning API keys on
a per-user basis (rather than one global plain-text environment
variable).
**Generating an API Key**
When making a `POST` request to the `/_users/` endpoint the inclusion
of the following in the request body will generate an API key for the
user:
```
POST /_users/
{
"apikey": true,
"auth": "admin",
"provider": "google",
"username": "test@example.com"
}
```
The response will look like this. _Note: This is the only opportunity
to retrieve the generated key. Keys are hashed in the database._
```js
{
"_ref": "/_users/dGVzdEBleGFtcGxlLmNvbUBnb29nbGU=",
"apikey": "dGVzdEBleGFtcGxlLmNvbUBnb29nbGU=:Z8EVBDN-M1T4EED-NH93XNJ-8J1374R",
"auth": "admin",
"provider": "google",
"username": "test@example.com"
}
```
**Revoking an API Key**
To remove a user's key `POST` again with `apikey: false`:
```
POST /_users/
{
"apikey": false,
"auth": "admin",
"provider": "google",
"username": "test@example.com"
}
```
The response will look like this.
```js
{
"_ref": "/_users/dGVzdEBleGFtcGxlLmNvbUBnb29nbGU=",
"apikey": null,
"auth": "admin",
"provider": "google",
"username": "test@example.com"
}
```
---
In addition to per-user API keys, this commit introduces a new
environment variable `CLAY_DISABLE_GLOBAL_ACCESS_KEY`. When set to a
"true" value (`t`, `true`, `1`) _only_ user-level keys will work.
This provides a "single-user-mode"-esq toggle for emergency recovery,
allows Clay sites to run in a secure mode, but maintains backward
compatibility for old API environment-based API keys where required or
where convenient.
Rather than having a variable to disable the global access key, this commit disables the global API key when `CLAY_ACCESS_KEY` is set to an empty value (or not set at all).
reubenson
reviewed
Oct 7, 2020
| }); | ||
|
|
||
| afterAll(() => { | ||
| process.env.CLAY_ACCESS_KEY = OLD_ACCESS_KEY; |
There was a problem hiding this comment.
not sure i understand what OLD_ACCESS_KEY is doing here, is it insufficient to just set process.env.CLAY_ACCESES_KEY as in L24 above?
Contributor
Author
There was a problem hiding this comment.
This is really just a pattern to preserve the original values of the environment before the suite was executed.
I think ideally we'd mock out the whole process.env for tests for reproducibility, but chose to stick close to what we had for this PR.
james-owen
reviewed
Oct 7, 2020
| * @returns {boolean} | ||
| */ | ||
| function isValidAPIKey(user, apikey) { | ||
| return bcrypt.compareSync(apikey, user.apikey); |
Member
There was a problem hiding this comment.
We might want to go with bcrypt.compare() to keep from blocking other requests. I'm not sure why were doing hashSync().
Contributor
Author
There was a problem hiding this comment.
Yeah good call, I'll dig into using the async methods instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit introduces a rudimentary approach to assigning API keys on
a per-user basis (rather than one global plain-text environment
variable).
Generating an API Key
When making a
POSTrequest to the/_users/endpoint the inclusionof the following in the request body will generate an API key for the
user:
The response will look like this. Note: This is the only opportunity
to retrieve the generated key. Keys are hashed in the database and
the hashed values are excluded from the GET API.
Revoking an API Key
To remove a user's key
POSTagain withapikey: false:The response will look like this.
In addition to per-user API keys, this commit introduces a newenvironment variableCLAY_DISABLE_GLOBAL_ACCESS_KEY. When set to a"true" value (t,true,1) only user-level keys will work.Edit: I replaced the
CLAY_DISABLE_GLOBAL_ACCESS_KEYidea with a simpler method.Set
CLAY_ACCESS_KEYto a value to enable the global access key.Unset
CLAY_ACCESS_KEYto disable it.This provides a "single-user-mode"-esq toggle for emergency recovery,
allows Clay sites to run in a secure mode, but maintains backward
compatibility for old API environment-based API keys where required or
where convenient.