Skip to content

Conversation

@cognifloyd
Copy link
Member

Based off of #615 #754 #809 #828 #829 #847 #848 #849 and #851

This change continues the refactor efforts initially introduced in the above PRs.

This adds a new auth package to the github.com/go-vela/server/api package.

This contains all the same handlers that existed previously but with each of them within their own file.

There are several top-level auth-related handlers, so I combined them into the same package, and I renamed some of the handlers.

Here is a summary of handler moves and renames:

Endpoint Old File Old Handler Name New File New Handler Name
GET /authenticate api/authenticate.go Authenticate api/auth/get_token.go GetAuthToken
GET /authenticate/:type[/:port] api/authenticate.go AuthenticateType api/auth/redirect.go GetAuthRedirect
POST /authenticate api/authenticate.go AuthenticateToken api/auth/post_token.go PostAuthToken
GET /login api/login.go Login api/auth/login.go Login
GET /logout api/logout.go Logout api/auth/logout.go Logout
GET /token-refresh api/token.go RefreshAccessToken api/auth/refresh.go RefreshAccessToken
GET /validate-token api/token.go ValidateServerToken api/auth/validate.go ValidateServerToken

NB: The commits are arranged to preserve git history (git blame) for all files. I suspect that the squash commit will erase that, but oh well - the history is at least preserved in this branch.

@cognifloyd cognifloyd self-assigned this May 19, 2023
@cognifloyd cognifloyd requested a review from a team as a code owner May 19, 2023 17:44
@cognifloyd cognifloyd requested review from ecrupper and jbrockopp May 19, 2023 17:47
@codecov
Copy link

codecov bot commented May 19, 2023

Codecov Report

Merging #853 (eb1c038) into main (a374e2b) will increase coverage by 1.07%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #853      +/-   ##
==========================================
+ Coverage   62.03%   63.10%   +1.07%     
==========================================
  Files         289      285       -4     
  Lines       15202    14944     -258     
==========================================
  Hits         9430     9430              
+ Misses       5326     5068     -258     
  Partials      446      446              

see 3 files with indirect coverage changes

Copy link
Contributor

@ecrupper ecrupper left a comment

Choose a reason for hiding this comment

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

Awesome — I like it. One thing I did notice that was from an old commit that would be nice to change with this refactor

Co-authored-by: Easton Crupper <[email protected]>
@cognifloyd cognifloyd requested a review from ecrupper May 19, 2023 18:31
@cognifloyd cognifloyd requested a review from plyr4 May 19, 2023 21:01
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.

4 participants