History

refactor auth interface to return error when verify failure (#22119)

This PR changed the Auth interface signature from `Verify(http *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) *user_model.User` to `Verify(http *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error)`. There is a new return argument `error` which means the verification condition matched but verify process failed, we should stop the auth process. Before this PR, when return a `nil` user, we don't know the reason why it returned `nil`. If the match condition is not satisfied or it verified failure? For these two different results, we should have different handler. If the match condition is not satisfied, we should try next auth method and if there is no more auth method, it's an anonymous user. If the condition matched but verify failed, the auth process should be stop and return immediately. This will fix #20563

Lunny Xiao committed 3 years ago

Commit ca67c5a1 parent 7cc7db715 files · +111 −79Browse files

Showing the changes to services/auth/httpsign.go (1 of 15 files). Show all

Unified · Split

services/auth/httpsign.go+7−7
@@ -39,10 +39,10 @@ func (h *HTTPSign) Name() string { · 38 unmodified lines
3939 // Verify extracts and validates HTTPsign from the Signature header of the request and returns
4040 // the corresponding user object on successful validation.
4141 // Returns nil if header is empty or validation fails.
42−func (h *HTTPSign) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) *user_model.User {
42+func (h *HTTPSign) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) {
4343 sigHead := req.Header.Get("Signature")
4444 if len(sigHead) == 0 {
45− return nil
45+ return nil, nil
4646 }
4747
4848 var (
@@ -53,14 +53,14 @@ func (h *HTTPSign) Verify(req *http.Request, w http.ResponseWriter, store DataSt · 4 unmodified lines
5353 if len(req.Header.Get("X-Ssh-Certificate")) != 0 {
5454 // Handle Signature signed by SSH certificates
5555 if len(setting.SSH.TrustedUserCAKeys) == 0 {
56− return nil
56+ return nil, nil
5757 }
5858
5959 publicKey, err = VerifyCert(req)
6060 if err != nil {
6161 log.Debug("VerifyCert on request from %s: failed: %v", req.RemoteAddr, err)
6262 log.Warn("Failed authentication attempt from %s", req.RemoteAddr)
63− return nil
63+ return nil, nil
6464 }
6565 } else {
6666 // Handle Signature signed by Public Key
@@ -68,21 +68,21 @@ func (h *HTTPSign) Verify(req *http.Request, w http.ResponseWriter, store DataSt · 1 unmodified line
6868 if err != nil {
6969 log.Debug("VerifyPubKey on request from %s: failed: %v", req.RemoteAddr, err)
7070 log.Warn("Failed authentication attempt from %s", req.RemoteAddr)
71− return nil
71+ return nil, nil
7272 }
7373 }
7474
7575 u, err := user_model.GetUserByID(req.Context(), publicKey.OwnerID)
7676 if err != nil {
7777 log.Error("GetUserByID: %v", err)
78− return nil
78+ return nil, err
7979 }
8080
8181 store.GetData()["IsApiToken"] = true
8282
8383 log.Trace("HTTP Sign: Logged in user %-v", u)
8484
85− return u
85+ return u, nil
8686 }
8787
8888 func VerifyPubKey(r *http.Request) (*asymkey_model.PublicKey, error) {