Authentication cleanup

[enrichman]

Note

This PR needs the new users from this helm-charts PR

• New default users helm-charts#184

It would work also without it, but I guess it's better to merge that before.

---

This is a cleanup/refactor made before the authorization spike. It should not change the behaviour of the API.

The main changes are in the internal/auth/auth.go.

Some unused structs and func were removed:

• PasswordAuth
• HashBcrypt()
• RandomPasswordAuth()

Then a new auth.User struct was added, and all the methods were adapted accordingly. This struct will match an Epinio User (or account maybe?), and it will be the one that will be returned from the different functions and package (I don't think we should move too much around with secrets and the kubernetes internal, generally speaking). This struct will be also the one that is going to be keep around in the context and session.

// User is a struct containing all the information of an Epinio User
type User struct {
	Username   string
	Password   string
	CreatedAt  time.Time
	Role       string
	Namespaces []string
}

So a NewUserFromSecret func was added, and the other GetUserAccounts, GetFirstUserAccount and GetUserSecretsByAge were updated accordingly.

It's probably easier to have a look at the result: https://github.com/epinio/epinio/blob/auth-cleanup/internal/auth/auth.go

Other important changes are in the internal/cli/server/server.go, so that now we are going to persist the auth.User in the session and context. I've also removed the BasicAuth header decoding, that can be done with just

username, password, ok = ctx.Request.BasicAuth()

[codecov-commenter]

Codecov Report

Merging #1403 (26afc0f) into main (c097bd4) will not change coverage.
The diff coverage is 0.00%.

@@          Coverage Diff          @@
##            main   #1403   +/-   ##
=====================================
  Coverage   7.75%   7.75%           
=====================================
  Files         69      69           
  Lines       7273    7273           
=====================================
  Hits         564     564           
  Misses      6669    6669           
  Partials      40      40           

Flag	Coverage Δ
unittests	7.75% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
internal/api/v1/application/create.go	0.00% <0.00%> (ø)
internal/api/v1/application/deploy.go	0.00% <0.00%> (ø)
internal/api/v1/application/importgit.go	0.00% <0.00%> (ø)
internal/api/v1/application/restart.go	0.00% <0.00%> (ø)
internal/api/v1/application/stage.go	0.00% <0.00%> (ø)
internal/api/v1/application/update.go	0.00% <0.00%> (ø)
internal/api/v1/application/upload.go	0.00% <0.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c097bd4...26afc0f. Read the comment docs.

[andreas-kupries]

Look ok.

[andreas-kupries]

looks ok.

wondering however if our tests actually check the various code paths of auth, i.e. try to trigger the various auth failures.

[enrichman]

Thanks @andreas-kupries! Actually yes, there are few tests that are checking the basic auth and session authentication (they failed me for a couple of things, so it was interesting to see that the auth is working fine!).

In the next PR I will add a couple of tests to check also the authorization.

[jimmykarily]

Comment is now outdated (sorry for the post-merge review)

[enrichman]

Ah, good catch, thanks. I will update it in the next PR!