Skip to content

Conversation

@ecrupper
Copy link
Contributor

@ecrupper ecrupper commented Feb 21, 2023

Server PR for Epic 1: Build Tokens.

Changes:

  • Conversion of router/middleware/token package to router/middleware/auth, which will now only retrieve auth headers from http requests, rather than generate/parse tokens
  • Creation of internal/token package, which features a manager that is set up in the context using various new flags added to main.go. In brief, it's a one-stop-shop for token minting and validating and offers configurability to a degree.
  • Additional claims added to Claims struct, including information about the token type as well as relevant fields for that token (build tokens have build ids and repo).
  • Removal of global permissions for agents
  • Worker endpoints used for check in (POST / PUT) have been put behind MustWorker() permissions check, which validates the symmetric token shared between server and worker
  • Build/Step/Service/Log update endpoints have been put behind MustBuildAccess() permissions check, which validates the build token, which is provided as a unique auth scope for each build.
  • Add GET endpoint for retrieving a build token, which validates the build's status and creates a build token
  • Addition of a private key, which will be seeded in the server environment, much like the secret, which will be used to sign and validate all tokens.
  • Updates made to MustSecretAdmin which will scope requests with build token auth based on the repo within the token
  • Claims will be established in the context for baseAPI endpoints before the user

depends on: go-vela/types#276

@ecrupper ecrupper requested a review from a team as a code owner February 21, 2023 22:08
wass3r
wass3r previously approved these changes Feb 21, 2023
@wass3r wass3r dismissed their stale review February 21, 2023 22:23

waiting for go-vela/types changes to merge in

@plyr4 plyr4 changed the title feat(build_tokens): server changes for build token implementation feat(auth): build token implementation Feb 22, 2023
@ecrupper ecrupper self-assigned this Feb 23, 2023
@codecov
Copy link

codecov bot commented Feb 23, 2023

Codecov Report

Merging #765 (411d800) into main (343d53e) will increase coverage by 0.31%.
The diff coverage is 65.31%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #765      +/-   ##
==========================================
+ Coverage   54.39%   54.70%   +0.31%     
==========================================
  Files         225      232       +7     
  Lines       16188    16392     +204     
==========================================
+ Hits         8806     8968     +162     
- Misses       6989     7024      +35     
- Partials      393      400       +7     
Impacted Files Coverage Δ
api/authenticate.go 0.00% <0.00%> (ø)
api/build.go 1.50% <0.00%> (-0.07%) ⬇️
api/secret.go 0.00% <0.00%> (ø)
api/token.go 0.00% <0.00%> (ø)
api/user.go 0.00% <0.00%> (ø)
database/user/table.go 100.00% <ø> (ø)
internal/token/mint.go 57.50% <57.50%> (ø)
internal/token/refresh.go 72.72% <72.72%> (ø)
router/middleware/perm/perm.go 62.33% <78.44%> (+16.12%) ⬆️
internal/token/compose.go 80.43% <80.43%> (ø)
... and 10 more

wass3r
wass3r previously approved these changes Feb 23, 2023
wass3r
wass3r previously approved these changes Feb 23, 2023
Copy link
Contributor

@jbrockopp jbrockopp left a comment

Choose a reason for hiding this comment

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

proposal?

@ecrupper
Copy link
Contributor Author

proposal?

go-vela/community#755

Copy link
Contributor

@jbrockopp jbrockopp left a comment

Choose a reason for hiding this comment

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

LGTM

@ecrupper ecrupper merged commit bff8352 into main Feb 23, 2023
@ecrupper ecrupper deleted the build-token/complete branch February 23, 2023 22:44
plyr4 added a commit that referenced this pull request Feb 24, 2023
plyr4 added a commit that referenced this pull request Feb 24, 2023
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.

7 participants