Fix the error message when the token is incorrect (#25701)
we refactored `userIDFromToken` for the token parsing part into a new function `parseToken`. `parseToken` returns the string `token` from request, and a boolean `ok` representing whether the token exists or not. So we can distinguish between token non-existence and token inconsistency in the `verfity` function, thus solving the problem of no proper error message when the token is inconsistent. close #24439 related #22119 --------- Co-authored-by: Jason Song <i@wolfogre.com> Co-authored-by: Giteabot <teabot@gitea.io>
This commit is contained in:
parent
2f31d2d56c
commit
491cc06ffe
|
@ -49,12 +49,22 @@ func (b *Group) Name() string {
|
||||||
// Verify extracts and validates
|
// Verify extracts and validates
|
||||||
func (b *Group) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) {
|
func (b *Group) Verify(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) (*user_model.User, error) {
|
||||||
// Try to sign in with each of the enabled plugins
|
// Try to sign in with each of the enabled plugins
|
||||||
|
var retErr error
|
||||||
for _, ssoMethod := range b.methods {
|
for _, ssoMethod := range b.methods {
|
||||||
user, err := ssoMethod.Verify(req, w, store, sess)
|
user, err := ssoMethod.Verify(req, w, store, sess)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
if retErr == nil {
|
||||||
|
retErr = err
|
||||||
|
}
|
||||||
|
// Try other methods if this one failed.
|
||||||
|
// Some methods may share the same protocol to detect if they are matched.
|
||||||
|
// For example, OAuth2 and conan.Auth both read token from "Authorization: Bearer <token>" header,
|
||||||
|
// If OAuth2 returns error, we should give conan.Auth a chance to try.
|
||||||
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// If any method returns a user, we can stop trying.
|
||||||
|
// Return the user and ignore any error returned by previous methods.
|
||||||
if user != nil {
|
if user != nil {
|
||||||
if store.GetData()["AuthedMethod"] == nil {
|
if store.GetData()["AuthedMethod"] == nil {
|
||||||
if named, ok := ssoMethod.(Named); ok {
|
if named, ok := ssoMethod.(Named); ok {
|
||||||
|
@ -65,5 +75,6 @@ func (b *Group) Verify(req *http.Request, w http.ResponseWriter, store DataStore
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return nil, nil
|
// If no method returns a user, return the error returned by the first method.
|
||||||
|
return nil, retErr
|
||||||
}
|
}
|
||||||
|
|
|
@ -59,31 +59,32 @@ func (o *OAuth2) Name() string {
|
||||||
return "oauth2"
|
return "oauth2"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// parseToken returns the token from request, and a boolean value
|
||||||
|
// representing whether the token exists or not
|
||||||
|
func parseToken(req *http.Request) (string, bool) {
|
||||||
|
_ = req.ParseForm()
|
||||||
|
// Check token.
|
||||||
|
if token := req.Form.Get("token"); token != "" {
|
||||||
|
return token, true
|
||||||
|
}
|
||||||
|
// Check access token.
|
||||||
|
if token := req.Form.Get("access_token"); token != "" {
|
||||||
|
return token, true
|
||||||
|
}
|
||||||
|
// check header token
|
||||||
|
if auHead := req.Header.Get("Authorization"); auHead != "" {
|
||||||
|
auths := strings.Fields(auHead)
|
||||||
|
if len(auths) == 2 && (auths[0] == "token" || strings.ToLower(auths[0]) == "bearer") {
|
||||||
|
return auths[1], true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return "", false
|
||||||
|
}
|
||||||
|
|
||||||
// userIDFromToken returns the user id corresponding to the OAuth token.
|
// userIDFromToken returns the user id corresponding to the OAuth token.
|
||||||
// It will set 'IsApiToken' to true if the token is an API token and
|
// It will set 'IsApiToken' to true if the token is an API token and
|
||||||
// set 'ApiTokenScope' to the scope of the access token
|
// set 'ApiTokenScope' to the scope of the access token
|
||||||
func (o *OAuth2) userIDFromToken(req *http.Request, store DataStore) int64 {
|
func (o *OAuth2) userIDFromToken(tokenSHA string, store DataStore) int64 {
|
||||||
_ = req.ParseForm()
|
|
||||||
|
|
||||||
// Check access token.
|
|
||||||
tokenSHA := req.Form.Get("token")
|
|
||||||
if len(tokenSHA) == 0 {
|
|
||||||
tokenSHA = req.Form.Get("access_token")
|
|
||||||
}
|
|
||||||
if len(tokenSHA) == 0 {
|
|
||||||
// Well, check with header again.
|
|
||||||
auHead := req.Header.Get("Authorization")
|
|
||||||
if len(auHead) > 0 {
|
|
||||||
auths := strings.Fields(auHead)
|
|
||||||
if len(auths) == 2 && (auths[0] == "token" || strings.ToLower(auths[0]) == "bearer") {
|
|
||||||
tokenSHA = auths[1]
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if len(tokenSHA) == 0 {
|
|
||||||
return 0
|
|
||||||
}
|
|
||||||
|
|
||||||
// Let's see if token is valid.
|
// Let's see if token is valid.
|
||||||
if strings.Contains(tokenSHA, ".") {
|
if strings.Contains(tokenSHA, ".") {
|
||||||
uid := CheckOAuthAccessToken(tokenSHA)
|
uid := CheckOAuthAccessToken(tokenSHA)
|
||||||
|
@ -129,10 +130,15 @@ func (o *OAuth2) Verify(req *http.Request, w http.ResponseWriter, store DataStor
|
||||||
return nil, nil
|
return nil, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
id := o.userIDFromToken(req, store)
|
token, ok := parseToken(req)
|
||||||
|
if !ok {
|
||||||
|
return nil, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
id := o.userIDFromToken(token, store)
|
||||||
|
|
||||||
if id <= 0 && id != -2 { // -2 means actions, so we need to allow it.
|
if id <= 0 && id != -2 { // -2 means actions, so we need to allow it.
|
||||||
return nil, nil
|
return nil, user_model.ErrUserNotExist{}
|
||||||
}
|
}
|
||||||
log.Trace("OAuth2 Authorization: Found token for user[%d]", id)
|
log.Trace("OAuth2 Authorization: Found token for user[%d]", id)
|
||||||
|
|
||||||
|
|
|
@ -41,6 +41,17 @@ func TestAPIUserReposNotLogin(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestAPIUserReposWithWrongToken(t *testing.T) {
|
||||||
|
defer tests.PrepareTestEnv(t)()
|
||||||
|
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
|
||||||
|
wrongToken := fmt.Sprintf("Bearer %s", "wrong_token")
|
||||||
|
req := NewRequestf(t, "GET", "/api/v1/users/%s/repos", user.Name)
|
||||||
|
req = addTokenAuthHeader(req, wrongToken)
|
||||||
|
resp := MakeRequest(t, req, http.StatusUnauthorized)
|
||||||
|
|
||||||
|
assert.Contains(t, resp.Body.String(), "user does not exist")
|
||||||
|
}
|
||||||
|
|
||||||
func TestAPISearchRepo(t *testing.T) {
|
func TestAPISearchRepo(t *testing.T) {
|
||||||
defer tests.PrepareTestEnv(t)()
|
defer tests.PrepareTestEnv(t)()
|
||||||
const keyword = "test"
|
const keyword = "test"
|
||||||
|
|
Loading…
Reference in a new issue