Page MenuHomePhabricator
Differential D6824

[identity] check that incoming requests contain valid auth token
ClosedPublic
Actions

Authored by varun on Feb 21 2023, 3:39 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 17, 3:59 AM
Unknown Object (File)
Wed, Apr 17, 3:59 AM
Unknown Object (File)
Wed, Apr 17, 3:59 AM
Unknown Object (File)
Wed, Apr 17, 3:59 AM
Unknown Object (File)
Wed, Apr 17, 3:59 AM
Unknown Object (File)
Wed, Apr 17, 3:59 AM
Unknown Object (File)
Wed, Apr 17, 3:58 AM
Unknown Object (File)
Wed, Apr 17, 3:53 AM
View All 83 Files
Subscribers
ashoat
atul

Details

Reviewers
• jon
bartek
tomek
Commits
rCOMMdbed4bc20c94: [identity] check that incoming requests contain valid auth token
Summary
  • statically initialize config so that we can access it from request interceptor
  • write interceptor function that compares authorization key from request metadata with the config token value
  • remove config param from other functions now that the config is static
Test Plan
  • first, ran the server and verified that the process exits if the AUTH_TOKEN variable is not present
  • then, set the env var and attempted to send requests to the server without any metadata; they were rejected as expected
  • lastly, sent a request with authorization key set in the metadata and received a successful response

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

varun created this revision.Feb 21 2023, 3:39 PM
Herald added subscribers: atul, ashoat. · View Herald TranscriptFeb 21 2023, 3:39 PM
varun updated this revision to Diff 22898.Feb 21 2023, 3:41 PM

remove .rdb file

varun added inline comments.Feb 21 2023, 3:43 PM
services/identity/src/interceptor.rs
6 ↗(On Diff #22898)

in the future, this function should compare the request metadata's auth token with the token value we have in DynamoDB for the user

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 21 2023, 3:43 PM
Harbormaster failed remote builds in B16740: Diff 22897!
Harbormaster failed remote builds in B16741: Diff 22898!
varun updated this revision to Diff 22904.Feb 21 2023, 5:14 PM

bump rust version

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 21 2023, 5:21 PM
Harbormaster failed remote builds in B16744: Diff 22904!
varun updated this revision to Diff 22905.Feb 21 2023, 6:16 PM

add default config for auth token

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 21 2023, 6:23 PM
Harbormaster failed remote builds in B16745: Diff 22905!
varun updated this revision to Diff 22906.Feb 21 2023, 6:40 PM

only load config if we're running the server

Harbormaster completed remote builds in B16746: Diff 22906.Feb 21 2023, 6:55 PM
varun requested review of this revision.Feb 21 2023, 6:55 PM
• jon added a comment.Feb 22 2023, 2:09 PM

We also need to do the other half of this on the client side

example intercepter https://github.com/hyperium/tonic/blob/4e578b5046804eb7abb2da80680366769b2a08fd/examples/src/interceptor/client.rs#L57

services/identity/src/constants.rs
40 ↗(On Diff #22906)

Can we name it something more distinct, like COMM_IDENTITY_AUTH_TOKEN. AUTH_TOKEN is likely to collide with something else.

• jon requested changes to this revision.Feb 22 2023, 2:09 PM
This revision now requires changes to proceed.Feb 22 2023, 2:09 PM
• jon added a child revision: D6825: [keyserver] update identity service client to add authorization metadata.Feb 22 2023, 2:18 PM
• jon mentioned this in D6825: [keyserver] update identity service client to add authorization metadata.
• jon added inline comments.Feb 22 2023, 2:22 PM
services/identity/src/constants.rs
40 ↗(On Diff #22906)

D6825 uses COMM_IDENTITY_SERVICE_AUTH_TOKEN which I think is also good for me.

varun updated this revision to Diff 23024.Feb 23 2023, 1:06 PM

address feedback

varun added a comment.Feb 23 2023, 1:07 PM
In D6824#203292, @jon wrote:

We also need to do the other half of this on the client side

example intercepter https://github.com/hyperium/tonic/blob/4e578b5046804eb7abb2da80680366769b2a08fd/examples/src/interceptor/client.rs#L57

next diff

services/identity/src/constants.rs
40 ↗(On Diff #22906)

yeah i'll change the actual env var name; gonna keep the variable name the same though since it's temporary and i doubt we'll have collisions in our code

Harbormaster completed remote builds in B16845: Diff 23024.Feb 23 2023, 1:22 PM
• jon accepted this revision.Feb 25 2023, 1:05 PM
This revision is now accepted and ready to land.Feb 25 2023, 1:05 PM
Closed by commit rCOMMdbed4bc20c94: [identity] check that incoming requests contain valid auth token (authored by varun). · Explain WhyFeb 27 2023, 7:25 AM
This revision was automatically updated to reflect the committed changes.
varun added a commit: rCOMMdbed4bc20c94: [identity] check that incoming requests contain valid auth token.

Revision Contents
Changeset List

PathSize
services/
identity/
1 line
2 lines
src/
19 lines
4 lines
17 lines
14 lines
14 lines
service/
2 lines
4 lines

Diff 23158

View Options

services/identity/Cargo.lock

Loading...
View Options

services/identity/Cargo.toml

Loading...
View Options

services/identity/Dockerfile

Loading...
View Options

services/identity/src/config.rs

Loading...
View Options

services/identity/src/constants.rs

Loading...
View Options

services/identity/src/interceptor.rs

Loading...
View Options

services/identity/src/main.rs

Loading...
View Options

services/identity/src/service.rs

Loading...
View Options

services/identity/src/service/login.rs

Loading...
View Options

services/identity/src/service/registration.rs

Loading...