Set Scheme to be OAuth2 explicitly for all users
Add Provider to Users authenticated via /me Signed-off-by: Michael de Sa <mjdesa@gmail.com>pull/2132/head
parent
c1b56f241d
commit
c994e8c5ac
|
@ -57,13 +57,13 @@ func (s *UsersStore) Get(ctx context.Context, q chronograf.UserQuery) (*chronogr
|
|||
return s.get(ctx, *q.ID)
|
||||
}
|
||||
|
||||
if q.Name != nil && q.Provider != nil {
|
||||
if q.Name != nil && q.Provider != nil && q.Scheme != nil {
|
||||
var user *chronograf.User
|
||||
err := s.each(ctx, func(u *chronograf.User) {
|
||||
if user != nil {
|
||||
return
|
||||
}
|
||||
if u.Name == *q.Name && u.Provider == *q.Provider {
|
||||
if u.Name == *q.Name && u.Provider == *q.Provider && u.Scheme == *q.Scheme {
|
||||
user = u
|
||||
}
|
||||
})
|
||||
|
@ -79,7 +79,7 @@ func (s *UsersStore) Get(ctx context.Context, q chronograf.UserQuery) (*chronogr
|
|||
return user, nil
|
||||
}
|
||||
|
||||
return nil, fmt.Errorf("must specify ID, or Name and Provider in UserQuery")
|
||||
return nil, fmt.Errorf("must specify either ID, or Name, Provider, and Scheme in UserQuery")
|
||||
}
|
||||
|
||||
// Add a new Users in the UsersStore.
|
||||
|
|
|
@ -45,11 +45,13 @@ func TestUsersStore_GetWithID(t *testing.T) {
|
|||
usr: &chronograf.User{
|
||||
Name: "billietta",
|
||||
Provider: "Google",
|
||||
Scheme: "OAuth2",
|
||||
},
|
||||
},
|
||||
want: &chronograf.User{
|
||||
Name: "billietta",
|
||||
Provider: "Google",
|
||||
Scheme: "OAuth2",
|
||||
},
|
||||
addFirst: true,
|
||||
},
|
||||
|
@ -82,7 +84,7 @@ func TestUsersStore_GetWithID(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestUsersStore_GetWithNameAndProvider(t *testing.T) {
|
||||
func TestUsersStore_GetWithNameProviderScheme(t *testing.T) {
|
||||
type args struct {
|
||||
ctx context.Context
|
||||
name string
|
||||
|
@ -103,6 +105,7 @@ func TestUsersStore_GetWithNameAndProvider(t *testing.T) {
|
|||
usr: &chronograf.User{
|
||||
Name: "billietta",
|
||||
Provider: "Google",
|
||||
Scheme: "OAuth2",
|
||||
},
|
||||
},
|
||||
wantErr: true,
|
||||
|
@ -114,11 +117,13 @@ func TestUsersStore_GetWithNameAndProvider(t *testing.T) {
|
|||
usr: &chronograf.User{
|
||||
Name: "billietta",
|
||||
Provider: "Google",
|
||||
Scheme: "OAuth2",
|
||||
},
|
||||
},
|
||||
want: &chronograf.User{
|
||||
Name: "billietta",
|
||||
Provider: "Google",
|
||||
Scheme: "OAuth2",
|
||||
},
|
||||
addFirst: true,
|
||||
},
|
||||
|
@ -144,6 +149,7 @@ func TestUsersStore_GetWithNameAndProvider(t *testing.T) {
|
|||
got, err := s.Get(tt.args.ctx, chronograf.UserQuery{
|
||||
Name: &tt.args.usr.Name,
|
||||
Provider: &tt.args.usr.Provider,
|
||||
Scheme: &tt.args.usr.Scheme,
|
||||
})
|
||||
if (err != nil) != tt.wantErr {
|
||||
t.Errorf("%q. UsersStore.Get() error = %v, wantErr %v", tt.name, err, tt.wantErr)
|
||||
|
|
|
@ -612,6 +612,7 @@ type UserQuery struct {
|
|||
ID *uint64
|
||||
Name *string
|
||||
Provider *string
|
||||
Scheme *string
|
||||
}
|
||||
|
||||
// UsersStore is the Storage and retrieval of authentication information
|
||||
|
|
|
@ -15,7 +15,7 @@ import (
|
|||
func AuthorizedToken(auth oauth2.Authenticator, logger chronograf.Logger, next http.Handler) http.HandlerFunc {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
log := logger.
|
||||
WithField("component", "auth").
|
||||
WithField("component", "token_auth").
|
||||
WithField("remote_addr", r.RemoteAddr).
|
||||
WithField("method", r.Method).
|
||||
WithField("url", r.URL)
|
||||
|
@ -83,8 +83,18 @@ func AuthorizedUser(
|
|||
Error(w, http.StatusUnauthorized, "User is not authorized", logger)
|
||||
return
|
||||
}
|
||||
scheme, err := getScheme(ctx)
|
||||
if err != nil {
|
||||
log.Error("Failed to retrieve scheme from context")
|
||||
Error(w, http.StatusUnauthorized, "User is not authorized", logger)
|
||||
return
|
||||
}
|
||||
|
||||
u, err := store.Get(ctx, chronograf.UserQuery{Name: &username, Provider: &provider})
|
||||
u, err := store.Get(ctx, chronograf.UserQuery{
|
||||
Name: &username,
|
||||
Provider: &provider,
|
||||
Scheme: &scheme,
|
||||
})
|
||||
if err != nil {
|
||||
log.Error("Failed to retrieve user")
|
||||
Error(w, http.StatusUnauthorized, "User is not authorized", logger)
|
||||
|
|
|
@ -3,6 +3,7 @@ package server_test
|
|||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
@ -130,6 +131,9 @@ func TestAuthorizedUser(t *testing.T) {
|
|||
fields: fields{
|
||||
UsersStore: &mocks.UsersStore{
|
||||
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
|
||||
if q.Name == nil || q.Provider == nil || q.Scheme == nil {
|
||||
return nil, fmt.Errorf("Invalid user query: missing Name, Provider, and/or Scheme")
|
||||
}
|
||||
return &chronograf.User{
|
||||
ID: 1337,
|
||||
Name: "billysteve",
|
||||
|
@ -156,6 +160,9 @@ func TestAuthorizedUser(t *testing.T) {
|
|||
fields: fields{
|
||||
UsersStore: &mocks.UsersStore{
|
||||
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
|
||||
if q.Name == nil || q.Provider == nil || q.Scheme == nil {
|
||||
return nil, fmt.Errorf("Invalid user query: missing Name, Provider, and/or Scheme")
|
||||
}
|
||||
return &chronograf.User{
|
||||
ID: 1337,
|
||||
Name: "billysteve",
|
||||
|
@ -182,6 +189,9 @@ func TestAuthorizedUser(t *testing.T) {
|
|||
fields: fields{
|
||||
UsersStore: &mocks.UsersStore{
|
||||
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
|
||||
if q.Name == nil || q.Provider == nil || q.Scheme == nil {
|
||||
return nil, fmt.Errorf("Invalid user query: missing Name, Provider, and/or Scheme")
|
||||
}
|
||||
return &chronograf.User{
|
||||
ID: 1337,
|
||||
Name: "billysteve",
|
||||
|
@ -208,6 +218,9 @@ func TestAuthorizedUser(t *testing.T) {
|
|||
fields: fields{
|
||||
UsersStore: &mocks.UsersStore{
|
||||
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
|
||||
if q.Name == nil || q.Provider == nil || q.Scheme == nil {
|
||||
return nil, fmt.Errorf("Invalid user query: missing Name, Provider, and/or Scheme")
|
||||
}
|
||||
return &chronograf.User{
|
||||
ID: 1337,
|
||||
Name: "billysteve",
|
||||
|
@ -234,6 +247,9 @@ func TestAuthorizedUser(t *testing.T) {
|
|||
fields: fields{
|
||||
UsersStore: &mocks.UsersStore{
|
||||
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
|
||||
if q.Name == nil || q.Provider == nil || q.Scheme == nil {
|
||||
return nil, fmt.Errorf("Invalid user query: missing Name, Provider, and/or Scheme")
|
||||
}
|
||||
return &chronograf.User{
|
||||
ID: 1337,
|
||||
Name: "billysteve",
|
||||
|
@ -260,6 +276,9 @@ func TestAuthorizedUser(t *testing.T) {
|
|||
fields: fields{
|
||||
UsersStore: &mocks.UsersStore{
|
||||
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
|
||||
if q.Name == nil || q.Provider == nil || q.Scheme == nil {
|
||||
return nil, fmt.Errorf("Invalid user query: missing Name, Provider, and/or Scheme")
|
||||
}
|
||||
return &chronograf.User{
|
||||
ID: 1337,
|
||||
Name: "billysteve",
|
||||
|
@ -286,6 +305,9 @@ func TestAuthorizedUser(t *testing.T) {
|
|||
fields: fields{
|
||||
UsersStore: &mocks.UsersStore{
|
||||
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
|
||||
if q.Name == nil || q.Provider == nil || q.Scheme == nil {
|
||||
return nil, fmt.Errorf("Invalid user query: missing Name, Provider, and/or Scheme")
|
||||
}
|
||||
return &chronograf.User{
|
||||
ID: 1337,
|
||||
Name: "billysteve",
|
||||
|
@ -312,6 +334,9 @@ func TestAuthorizedUser(t *testing.T) {
|
|||
fields: fields{
|
||||
UsersStore: &mocks.UsersStore{
|
||||
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
|
||||
if q.Name == nil || q.Provider == nil || q.Scheme == nil {
|
||||
return nil, fmt.Errorf("Invalid user query: missing Name, Provider, and/or Scheme")
|
||||
}
|
||||
return &chronograf.User{
|
||||
ID: 1337,
|
||||
Name: "billysteve",
|
||||
|
@ -338,6 +363,9 @@ func TestAuthorizedUser(t *testing.T) {
|
|||
fields: fields{
|
||||
UsersStore: &mocks.UsersStore{
|
||||
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
|
||||
if q.Name == nil || q.Provider == nil || q.Scheme == nil {
|
||||
return nil, fmt.Errorf("Invalid user query: missing Name, Provider, and/or Scheme")
|
||||
}
|
||||
return &chronograf.User{
|
||||
ID: 1337,
|
||||
Name: "billysteve",
|
||||
|
@ -364,6 +392,9 @@ func TestAuthorizedUser(t *testing.T) {
|
|||
fields: fields{
|
||||
UsersStore: &mocks.UsersStore{
|
||||
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
|
||||
if q.Name == nil || q.Provider == nil || q.Scheme == nil {
|
||||
return nil, fmt.Errorf("Invalid user query: missing Name, Provider, and/or Scheme")
|
||||
}
|
||||
return &chronograf.User{
|
||||
ID: 1337,
|
||||
Name: "billysteve",
|
||||
|
@ -388,6 +419,9 @@ func TestAuthorizedUser(t *testing.T) {
|
|||
fields: fields{
|
||||
UsersStore: &mocks.UsersStore{
|
||||
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
|
||||
if q.Name == nil || q.Provider == nil || q.Scheme == nil {
|
||||
return nil, fmt.Errorf("Invalid user query: missing Name, Provider, and/or Scheme")
|
||||
}
|
||||
return &chronograf.User{
|
||||
ID: 1337,
|
||||
Name: "billysteve",
|
||||
|
@ -412,6 +446,9 @@ func TestAuthorizedUser(t *testing.T) {
|
|||
fields: fields{
|
||||
UsersStore: &mocks.UsersStore{
|
||||
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
|
||||
if q.Name == nil || q.Provider == nil || q.Scheme == nil {
|
||||
return nil, fmt.Errorf("Invalid user query: missing Name, Provider, and/or Scheme")
|
||||
}
|
||||
return &chronograf.User{
|
||||
ID: 1337,
|
||||
Name: "billysteve",
|
||||
|
@ -436,6 +473,9 @@ func TestAuthorizedUser(t *testing.T) {
|
|||
fields: fields{
|
||||
UsersStore: &mocks.UsersStore{
|
||||
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
|
||||
if q.Name == nil || q.Provider == nil || q.Scheme == nil {
|
||||
return nil, fmt.Errorf("Invalid user query: missing Name, Provider, and/or Scheme")
|
||||
}
|
||||
return &chronograf.User{
|
||||
ID: 1337,
|
||||
Name: "billysteve",
|
||||
|
@ -464,6 +504,9 @@ func TestAuthorizedUser(t *testing.T) {
|
|||
fields: fields{
|
||||
UsersStore: &mocks.UsersStore{
|
||||
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
|
||||
if q.Name == nil || q.Provider == nil || q.Scheme == nil {
|
||||
return nil, fmt.Errorf("Invalid user query: missing Name, Provider, and/or Scheme")
|
||||
}
|
||||
return &chronograf.User{
|
||||
ID: 1337,
|
||||
Name: "billysteve",
|
||||
|
@ -492,6 +535,9 @@ func TestAuthorizedUser(t *testing.T) {
|
|||
fields: fields{
|
||||
UsersStore: &mocks.UsersStore{
|
||||
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
|
||||
if q.Name == nil || q.Provider == nil || q.Scheme == nil {
|
||||
return nil, fmt.Errorf("Invalid user query: missing Name, Provider, and/or Scheme")
|
||||
}
|
||||
return &chronograf.User{
|
||||
ID: 1337,
|
||||
Name: "billysteve",
|
||||
|
@ -538,7 +584,7 @@ func TestAuthorizedUser(t *testing.T) {
|
|||
fn(w, r)
|
||||
|
||||
if authorized != tt.authorized {
|
||||
t.Errorf("%q. AuthorizedUser() = %v, expected %v", tt.name, tt.authorized, authorized)
|
||||
t.Errorf("%q. AuthorizedUser() = %v, expected %v", tt.name, authorized, tt.authorized)
|
||||
}
|
||||
|
||||
})
|
||||
|
|
25
server/me.go
25
server/me.go
|
@ -58,6 +58,13 @@ func getProvider(ctx context.Context) (string, error) {
|
|||
return principal.Issuer, nil
|
||||
}
|
||||
|
||||
// TODO: This Scheme value is hard-coded temporarily since we only currently
|
||||
// support OAuth2. This hard-coding should be removed whenever we add
|
||||
// support for other authentication schemes.
|
||||
func getScheme(ctx context.Context) (string, error) {
|
||||
return "OAuth2", nil
|
||||
}
|
||||
|
||||
func getPrincipal(ctx context.Context) (oauth2.Principal, error) {
|
||||
principal, ok := ctx.Value(oauth2.PrincipalKey).(oauth2.Principal)
|
||||
if !ok {
|
||||
|
@ -87,8 +94,17 @@ func (s *Service) Me(w http.ResponseWriter, r *http.Request) {
|
|||
invalidData(w, err, s.Logger)
|
||||
return
|
||||
}
|
||||
scheme, err := getScheme(ctx)
|
||||
if err != nil {
|
||||
invalidData(w, err, s.Logger)
|
||||
return
|
||||
}
|
||||
|
||||
usr, err := s.UsersStore.Get(ctx, chronograf.UserQuery{Name: &username, Provider: &provider})
|
||||
usr, err := s.UsersStore.Get(ctx, chronograf.UserQuery{
|
||||
Name: &username,
|
||||
Provider: &provider,
|
||||
Scheme: &scheme,
|
||||
})
|
||||
if err != nil && err != chronograf.ErrUserNotFound {
|
||||
unknownErrorWithMessage(w, err, s.Logger)
|
||||
return
|
||||
|
@ -102,7 +118,12 @@ func (s *Service) Me(w http.ResponseWriter, r *http.Request) {
|
|||
|
||||
// Because we didnt find a user, making a new one
|
||||
user := &chronograf.User{
|
||||
Name: username,
|
||||
Name: username,
|
||||
Provider: provider,
|
||||
// TODO: This Scheme value is hard-coded temporarily since we only currently
|
||||
// support OAuth2. This hard-coding should be removed whenever we add
|
||||
// support for other authentication schemes.
|
||||
Scheme: "OAuth2",
|
||||
}
|
||||
|
||||
newUser, err := s.UsersStore.Add(ctx, user)
|
||||
|
|
|
@ -46,10 +46,13 @@ func TestService_Me(t *testing.T) {
|
|||
Logger: log.New(log.DebugLevel),
|
||||
UsersStore: &mocks.UsersStore{
|
||||
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
|
||||
if q.Name == nil || q.Provider == nil || q.Scheme == nil {
|
||||
return nil, fmt.Errorf("Invalid user query: missing Name, Provider, and/or Scheme")
|
||||
}
|
||||
return &chronograf.User{
|
||||
Name: "me",
|
||||
Provider: "GitHub",
|
||||
Passwd: "hunter2",
|
||||
Scheme: "OAuth2",
|
||||
}, nil
|
||||
},
|
||||
},
|
||||
|
@ -60,7 +63,7 @@ func TestService_Me(t *testing.T) {
|
|||
},
|
||||
wantStatus: http.StatusOK,
|
||||
wantContentType: "application/json",
|
||||
wantBody: `{"name":"me","password":"hunter2","provider":"GitHub","links":{"self":"/chronograf/v1/users/me"}}
|
||||
wantBody: `{"name":"me","provider":"GitHub","scheme":"OAuth2","links":{"self":"/chronograf/v1/users/me"}}
|
||||
`,
|
||||
},
|
||||
{
|
||||
|
@ -74,6 +77,9 @@ func TestService_Me(t *testing.T) {
|
|||
Logger: log.New(log.DebugLevel),
|
||||
UsersStore: &mocks.UsersStore{
|
||||
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
|
||||
if q.Name == nil || q.Provider == nil || q.Scheme == nil {
|
||||
return nil, fmt.Errorf("Invalid user query: missing Name, Provider, and/or Scheme")
|
||||
}
|
||||
return nil, chronograf.ErrUserNotFound
|
||||
},
|
||||
AddF: func(ctx context.Context, u *chronograf.User) (*chronograf.User, error) {
|
||||
|
@ -87,7 +93,7 @@ func TestService_Me(t *testing.T) {
|
|||
},
|
||||
wantStatus: http.StatusOK,
|
||||
wantContentType: "application/json",
|
||||
wantBody: `{"name":"secret","links":{"self":"/chronograf/v1/users/secret"}}
|
||||
wantBody: `{"name":"secret","provider":"Auth0","scheme":"OAuth2","links":{"self":"/chronograf/v1/users/secret"}}
|
||||
`,
|
||||
},
|
||||
{
|
||||
|
|
|
@ -29,6 +29,11 @@ func (r *userRequest) ValidCreate() error {
|
|||
if r.Scheme == "" {
|
||||
return fmt.Errorf("Scheme required on Chronograf User request body")
|
||||
}
|
||||
|
||||
// TODO: This Scheme value is hard-coded temporarily since we only currently
|
||||
// support OAuth2. This hard-coding should be removed whenever we add
|
||||
// support for other authentication schemes.
|
||||
r.Scheme = "OAuth2"
|
||||
return r.ValidRoles()
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue