feat(authorization): Add bcrypt password support to v1 authorizations #19840
Conversation
This file contains 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
This commit extends the `v1/authorization` package to support
passwords associated with a token.
The summary of changes include:
* authorization.Service implements influxdb.PasswordsService
* Setting passwords for authorizations
* Verifying (comparing) passwords for a given authorization
* A service to cache comparing passwords, using a weaker hash
that will live in memory only. This implementation is copied
from InfluxDB 1.x
* Extended HTTP service to set a password using
/private/legacy/authorizations/{id}/password
Closes #
stuartcarnie
force-pushed
the
sgc/issues/19837
branch
from
1f6f433
to
40b07d7
Compare
stuartcarnie
requested review from
a team and
yquansah
and removed request for
a team
yquansah
approved these changes
Oct 28, 2020
| inner influxdb.PasswordsService | ||
|
|
||
| mu sync.RWMutex // protects concurrent access to authCache | ||
| authCache map[influxdb.ID]authUser |
There was a problem hiding this comment.
What are the bounds on this in memory cache.. or do we not need to worry about it growing unbounded?
There was a problem hiding this comment.
Good question – this is only for unique user name tokens, so we'd expect this to be probably dozens at most
stuartcarnie
added a commit
that referenced
this pull request
Oct 28, 2020
Remove remnants of previous token implementation * As of #19840, V1 API tokens will be authorized using bcrypt passwords
stuartcarnie
added a commit
that referenced
this pull request
Oct 29, 2020
Remove remnants of previous token implementation * As of #19840, V1 API tokens will be authorized using bcrypt passwords
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.
Closes #19837
This commit extends the
v1/authorizationpackage to support passwords associated with a token using bcrypt hashing. This feature is added to support upgrading InfluxDB 1.x users and using their existing passwords. A subsequent PR will utilize these APIs to authorize requests to the/queryand/writeHTTP endpoints by verifying (comparing) a provided password for a given authorization. These passwords will either be supplied via anAuthorizationheader using theBASICauthentication scheme or via theu(user) andp(password) query parameters.The
influxd upgradecommand will be extended to copy the users frommeta.dband store the bcrypted version of the user's password so existing clients will not require changes.The summary of changes include:
authorization.Serviceimplementsinfluxdb.PasswordsServiceinfluxdb.PasswordsService:/query,/write)influxdb.PasswordsServicethat implements a cache when comparing passwords. These are salted and stored in-memory using a SHA256 hash.NOTE: This implementation is essentially copied from InfluxDB 1.x.
/private/legacy/authorizations/{id}/passwordAs this is a private API, the CHANGELOG is not updated.