Fix RouteMatchesPrincipal if org isnt on principal

pull/2733/head
Michael Desa 2018-01-17 12:42:32 -05:00
parent 23a4c34a17
commit 8fa2eabdb3
4 changed files with 115 additions and 1 deletions

View File

@ -1893,6 +1893,74 @@ func TestServer(t *testing.T) {
"code": 401,
"message": "user cannot modify their own SuperAdmin status"
}
`,
},
},
{
name: "GET /organization/default/users",
subName: "Organization not set explicitly on principal",
fields: fields{
Config: &chronograf.Config{
Auth: chronograf.AuthConfig{
SuperAdminNewUsers: false,
},
},
Organizations: []chronograf.Organization{},
Users: []chronograf.User{
{
ID: 1, // This is artificial, but should be reflective of the users actual ID
Name: "billibob",
Provider: "github",
Scheme: "oauth2",
SuperAdmin: true,
Roles: []chronograf.Role{
{
Name: "admin",
Organization: "default",
},
},
},
},
},
args: args{
server: &server.Server{
GithubClientID: "not empty",
GithubClientSecret: "not empty",
},
method: "GET",
path: "/chronograf/v1/organizations/default/users",
principal: oauth2.Principal{
Organization: "",
Subject: "billibob",
Issuer: "github",
},
},
wants: wants{
statusCode: 200,
body: `
{
"links": {
"self": "/chronograf/v1/organizations/default/users"
},
"users": [
{
"links": {
"self": "/chronograf/v1/organizations/default/users/1"
},
"id": "1",
"name": "billibob",
"provider": "github",
"scheme": "oauth2",
"superAdmin": true,
"roles": [
{
"name": "admin",
"organization": "default"
}
]
}
]
}
`,
},
},

View File

@ -10,6 +10,7 @@ import (
// RouteMatchesPrincipal checks that the organization on context matches the organization
// in the route.
func RouteMatchesPrincipal(
store DataStore,
useAuth bool,
logger chronograf.Logger,
next http.HandlerFunc,
@ -35,6 +36,16 @@ func RouteMatchesPrincipal(
return
}
if p.Organization == "" {
defaultOrg, err := store.Organizations(ctx).DefaultOrganization(ctx)
if err != nil {
log.Error("Failed to look up default organization")
Error(w, http.StatusForbidden, "User is not authorized", logger)
return
}
p.Organization = defaultOrg.ID
}
if orgID != p.Organization {
log.Error("Route organization does not match the organization on principal")
Error(w, http.StatusForbidden, "User is not authorized", logger)

View File

@ -9,12 +9,14 @@ import (
"github.com/bouk/httprouter"
"github.com/influxdata/chronograf"
"github.com/influxdata/chronograf/log"
"github.com/influxdata/chronograf/mocks"
"github.com/influxdata/chronograf/oauth2"
)
func TestRouteMatchesPrincipal(t *testing.T) {
type fields struct {
Logger chronograf.Logger
OrganizationsStore chronograf.OrganizationsStore
Logger chronograf.Logger
}
type args struct {
useAuth bool
@ -34,6 +36,13 @@ func TestRouteMatchesPrincipal(t *testing.T) {
name: "route matches request params",
fields: fields{
Logger: log.New(log.DebugLevel),
OrganizationsStore: &mocks.OrganizationsStore{
DefaultOrganizationF: func(ctx context.Context) (*chronograf.Organization, error) {
return &chronograf.Organization{
ID: "default",
}, nil
},
},
},
args: args{
useAuth: true,
@ -57,6 +66,13 @@ func TestRouteMatchesPrincipal(t *testing.T) {
name: "route does not match request params",
fields: fields{
Logger: log.New(log.DebugLevel),
OrganizationsStore: &mocks.OrganizationsStore{
DefaultOrganizationF: func(ctx context.Context) (*chronograf.Organization, error) {
return &chronograf.Organization{
ID: "default",
}, nil
},
},
},
args: args{
useAuth: true,
@ -80,6 +96,13 @@ func TestRouteMatchesPrincipal(t *testing.T) {
name: "missing principal",
fields: fields{
Logger: log.New(log.DebugLevel),
OrganizationsStore: &mocks.OrganizationsStore{
DefaultOrganizationF: func(ctx context.Context) (*chronograf.Organization, error) {
return &chronograf.Organization{
ID: "default",
}, nil
},
},
},
args: args{
useAuth: true,
@ -99,6 +122,13 @@ func TestRouteMatchesPrincipal(t *testing.T) {
name: "not using auth",
fields: fields{
Logger: log.New(log.DebugLevel),
OrganizationsStore: &mocks.OrganizationsStore{
DefaultOrganizationF: func(ctx context.Context) (*chronograf.Organization, error) {
return &chronograf.Organization{
ID: "default",
}, nil
},
},
},
args: args{
useAuth: false,
@ -122,11 +152,15 @@ func TestRouteMatchesPrincipal(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
store := &mocks.Store{
OrganizationsStore: tt.fields.OrganizationsStore,
}
var matches bool
next := func(w http.ResponseWriter, r *http.Request) {
matches = true
}
fn := RouteMatchesPrincipal(
store,
tt.args.useAuth,
tt.fields.Logger,
next,

View File

@ -121,6 +121,7 @@ func NewMux(opts MuxOpts, service Service) http.Handler {
ensureOrgMatches := func(next http.HandlerFunc) http.HandlerFunc {
return RouteMatchesPrincipal(
service.Store,
opts.UseAuth,
opts.Logger,
next,