diff options
author | George Goldberg <george@gberg.me> | 2018-02-06 17:25:53 +0000 |
---|---|---|
committer | George Goldberg <george@gberg.me> | 2018-02-06 17:25:53 +0000 |
commit | 7941c30117efe1b957ac0458c2f0479e3824196d (patch) | |
tree | df791632a9dc790a6f73dec53aae3ba919ebda63 /api | |
parent | e1cd64613591cf5a990442a69ebf188258bd0cb5 (diff) | |
parent | 034dbc07e3068c482e654b6a1a8fcbe4b01c44f3 (diff) | |
download | chat-7941c30117efe1b957ac0458c2f0479e3824196d.tar.gz chat-7941c30117efe1b957ac0458c2f0479e3824196d.tar.bz2 chat-7941c30117efe1b957ac0458c2f0479e3824196d.zip |
Merge branch 'master' into advanced-permissions-phase-1
Diffstat (limited to 'api')
-rw-r--r-- | api/context.go | 45 | ||||
-rw-r--r-- | api/user.go | 2 | ||||
-rw-r--r-- | api/user_test.go | 45 |
3 files changed, 55 insertions, 37 deletions
diff --git a/api/context.go b/api/context.go index 34a87e633..84967659d 100644 --- a/api/context.go +++ b/api/context.go @@ -114,38 +114,14 @@ func (h handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { metrics.IncrementHttpRequest() } - token := "" - isTokenFromQueryString := false - - // Attempt to parse token out of the header - authHeader := r.Header.Get(model.HEADER_AUTH) - if len(authHeader) > 6 && strings.ToUpper(authHeader[0:6]) == model.HEADER_BEARER { - // Default session token - token = authHeader[7:] - - } else if len(authHeader) > 5 && strings.ToLower(authHeader[0:5]) == model.HEADER_TOKEN { - // OAuth token - token = authHeader[6:] - } - - // Attempt to parse the token from the cookie - if len(token) == 0 { - if cookie, err := r.Cookie(model.SESSION_COOKIE_TOKEN); err == nil { - token = cookie.Value - - if (h.requireSystemAdmin || h.requireUser) && !h.trustRequester { - if r.Header.Get(model.HEADER_REQUESTED_WITH) != model.HEADER_REQUESTED_WITH_XML { - c.Err = model.NewAppError("ServeHTTP", "api.context.session_expired.app_error", nil, "token="+token+" Appears to be a CSRF attempt", http.StatusUnauthorized) - token = "" - } - } - } - } + token, tokenLocation := app.ParseAuthTokenFromRequest(r) - // Attempt to parse token out of the query string - if len(token) == 0 { - token = r.URL.Query().Get("access_token") - isTokenFromQueryString = true + // CSRF Check + if tokenLocation == app.TokenLocationCookie && (h.requireSystemAdmin || h.requireUser) && !h.trustRequester { + if r.Header.Get(model.HEADER_REQUESTED_WITH) != model.HEADER_REQUESTED_WITH_XML { + c.Err = model.NewAppError("ServeHTTP", "api.context.session_expired.app_error", nil, "token="+token+" Appears to be a CSRF attempt", http.StatusUnauthorized) + token = "" + } } c.SetSiteURLHeader(app.GetProtocol(r) + "://" + r.Host) @@ -175,11 +151,16 @@ func (h handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if h.requireUser || h.requireSystemAdmin { c.Err = model.NewAppError("ServeHTTP", "api.context.session_expired.app_error", nil, "token="+token, http.StatusUnauthorized) } - } else if !session.IsOAuth && isTokenFromQueryString { + } else if !session.IsOAuth && tokenLocation == app.TokenLocationQueryString { c.Err = model.NewAppError("ServeHTTP", "api.context.token_provided.app_error", nil, "token="+token, http.StatusUnauthorized) } else { c.Session = *session } + + // Rate limit by UserID + if c.App.Srv.RateLimiter != nil && c.App.Srv.RateLimiter.UserIdRateLimit(c.Session.UserId, w) { + return + } } if h.isApi || h.isTeamIndependent { diff --git a/api/user.go b/api/user.go index ae8a6cab6..a2e06c013 100644 --- a/api/user.go +++ b/api/user.go @@ -1159,7 +1159,7 @@ func completeSaml(c *Context, w http.ResponseWriter, r *http.Request) { } return } else { - if err := c.App.CheckUserAdditionalAuthenticationCriteria(user, ""); err != nil { + if err := c.App.CheckUserAllAuthenticationCriteria(user, ""); err != nil { c.Err = err c.Err.StatusCode = http.StatusFound return diff --git a/api/user_test.go b/api/user_test.go index b74039ce9..8d6aad22b 100644 --- a/api/user_test.go +++ b/api/user_test.go @@ -16,6 +16,8 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + "github.com/mattermost/mattermost-server/app" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/store" @@ -187,7 +189,7 @@ func TestLogin(t *testing.T) { } if _, err := Client.Login(ruser2.Data.(*model.User).Email, user2.Password); err != nil { - t.Fatal("From verfied hash") + t.Fatal("From verified hash") } Client.AuthToken = authToken @@ -199,14 +201,49 @@ func TestLogin(t *testing.T) { Password: "passwd1", AuthService: model.USER_AUTH_SERVICE_LDAP, } - user3 = Client.Must(Client.CreateUser(user3, "")).Data.(*model.User) - store.Must(th.App.Srv.Store.User().VerifyEmail(user3.Id)) + ruser3 := Client.Must(Client.CreateUser(user3, "")).Data.(*model.User) + store.Must(th.App.Srv.Store.User().VerifyEmail(ruser3.Id)) - if _, err := Client.Login(user3.Id, user3.Password); err == nil { + if _, err := Client.Login(ruser3.Id, user3.Password); err == nil { t.Fatal("AD/LDAP user should not be able to log in with AD/LDAP disabled") } } +func TestLoginUnverifiedEmail(t *testing.T) { + assert := assert.New(t) + + th := Setup().InitBasic() + defer th.TearDown() + + Client := th.BasicClient + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.EmailSettings.EnableSignInWithEmail = true + cfg.EmailSettings.RequireEmailVerification = true + }) + + Client.Logout() + + user := &model.User{ + Email: strings.ToLower(model.NewId()) + "success+test@simulator.amazonses.com", + Nickname: "Corey Hulen", + Username: "corey" + model.NewId(), + Password: "passwd1", + EmailVerified: false, + } + user = Client.Must(Client.CreateUser(user, "")).Data.(*model.User) + + _, err := Client.Login(user.Email, user.Password+"invalid") + if assert.NotNil(err) { + assert.Equal("api.user.check_user_password.invalid.app_error", err.Id) + } + + _, err = Client.Login(user.Email, "passwd1") + if assert.NotNil(err) { + assert.Equal("api.user.login.not_verified.app_error", err.Id) + } +} + func TestLoginByLdap(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() |