From 24cc1cabb8e6ac5ab9f03dc3366384f864f8445a Mon Sep 17 00:00:00 2001 From: Alirie Gray Date: Fri, 8 May 2020 10:00:39 -0700 Subject: [PATCH] feat: add feature flag for refactored authorization package (#17944) --- authorization/error.go | 2 +- authorization/http_server.go | 90 +-- authorization/http_server_test.go | 937 +++++++++++++++++++++-- authorization/middleware_auth.go | 20 +- authorization/mock_tenant.go | 35 + authorization/service.go | 4 +- cmd/influxd/launcher/launcher.go | 27 +- flags.yml | 7 + http/api_handler.go | 4 - kit/feature/list.go | 16 + kit/transport/http/feature_controller.go | 39 + 11 files changed, 1036 insertions(+), 145 deletions(-) create mode 100644 authorization/mock_tenant.go create mode 100644 kit/transport/http/feature_controller.go diff --git a/authorization/error.go b/authorization/error.go index b23872afc0..0f1174d5d4 100644 --- a/authorization/error.go +++ b/authorization/error.go @@ -19,7 +19,7 @@ var ( Msg: "authorization not found", } - // NotUniqueIDError is used when ... + // NotUniqueIDError occurs when attempting to create an Authorization with an ID that already belongs to another one NotUniqueIDError = &influxdb.Error{ Code: influxdb.EConflict, Msg: "ID already exists", diff --git a/authorization/http_server.go b/authorization/http_server.go index 02dd4457b5..8c3046e28b 100644 --- a/authorization/http_server.go +++ b/authorization/http_server.go @@ -10,21 +10,30 @@ import ( "github.com/go-chi/chi" "github.com/go-chi/chi/middleware" "github.com/influxdata/influxdb/v2" + icontext "github.com/influxdata/influxdb/v2/context" kithttp "github.com/influxdata/influxdb/v2/kit/transport/http" "go.uber.org/zap" ) +// TenantService is used to look up the Organization and User for an Authorization +type TenantService interface { + FindOrganizationByID(ctx context.Context, id influxdb.ID) (*influxdb.Organization, error) + FindOrganization(ctx context.Context, filter influxdb.OrganizationFilter) (*influxdb.Organization, error) + FindUserByID(ctx context.Context, id influxdb.ID) (*influxdb.User, error) + FindUser(ctx context.Context, filter influxdb.UserFilter) (*influxdb.User, error) +} + type AuthHandler struct { chi.Router api *kithttp.API log *zap.Logger authSvc influxdb.AuthorizationService lookupService influxdb.LookupService - tenantService influxdb.TenantService + tenantService TenantService } // NewHTTPAuthHandler constructs a new http server. -func NewHTTPAuthHandler(log *zap.Logger, authService influxdb.AuthorizationService, tenantService influxdb.TenantService, lookupService influxdb.LookupService) *AuthHandler { +func NewHTTPAuthHandler(log *zap.Logger, authService influxdb.AuthorizationService, tenantService TenantService, lookupService influxdb.LookupService) *AuthHandler { h := &AuthHandler{ api: kithttp.NewAPI(kithttp.WithLog(log)), log: log, @@ -69,9 +78,19 @@ func (h *AuthHandler) handlePostAuthorization(w http.ResponseWriter, r *http.Req h.api.Err(w, err) return } - // We can assume we have a User ID because if the request does not provide one, then the authorizer - // middleware gets it from the context - auth := a.toInfluxdb(*a.UserID) + + user, err := getAuthorizedUser(r, h.tenantService) + if err != nil { + h.api.Err(w, influxdb.ErrUnableToCreateToken) + return + } + + userID := user.ID + if a.UserID != nil && a.UserID.Valid() { + userID = *a.UserID + } + + auth := a.toInfluxdb(userID) if err := h.authSvc.CreateAuthorization(ctx, auth); err != nil { h.api.Err(w, err) @@ -88,12 +107,24 @@ func (h *AuthHandler) handlePostAuthorization(w http.ResponseWriter, r *http.Req resp, err := h.newAuthResponse(ctx, auth, perms) if err != nil { - h.api.Err(w, influxdb.ErrUnableToCreateToken) + h.api.Err(w, err) + return } h.api.Respond(w, http.StatusCreated, resp) } +func getAuthorizedUser(r *http.Request, ts TenantService) (*influxdb.User, error) { + ctx := r.Context() + + a, err := icontext.GetAuthorizer(ctx) + if err != nil { + return nil, err + } + + return ts.FindUserByID(ctx, a.GetUserID()) +} + type postAuthorizationRequest struct { Status influxdb.Status `json:"status"` OrgID influxdb.ID `json:"orgID"` @@ -457,30 +488,6 @@ func (h *AuthHandler) handleGetAuthorization(w http.ResponseWriter, r *http.Requ h.api.Respond(w, http.StatusOK, resp) } -// type getAuthorizationRequest struct { -// ID influxdb.ID -// } - -// func decodeGetAuthorizationRequest(ctx context.Context, r *http.Request) (*getAuthorizationRequest, error) { -// params := httprouter.ParamsFromContext(ctx) -// id := params.ByName("id") -// if id == "" { -// return nil, &influxdb.Error{ -// Code: influxdb.EInvalid, -// Msg: "url missing id", -// } -// } - -// var i influxdb.ID -// if err := i.DecodeFromString(id); err != nil { -// return nil, err -// } - -// return &getAuthorizationRequest{ -// ID: i, -// }, nil -// } - // handleUpdateAuthorization is the HTTP handler for the PATCH /api/v2/authorizations/:id route that updates the authorization's status and desc. func (h *AuthHandler) handleUpdateAuthorization(w http.ResponseWriter, r *http.Request) { ctx := r.Context() @@ -543,37 +550,20 @@ func decodeUpdateAuthorizationRequest(ctx context.Context, r *http.Request) (*up // handleDeleteAuthorization is the HTTP handler for the DELETE /api/v2/authorizations/:id route. func (h *AuthHandler) handleDeleteAuthorization(w http.ResponseWriter, r *http.Request) { - ctx := r.Context() - req, err := decodeDeleteAuthorizationRequest(ctx, r) + id, err := influxdb.IDFromString(chi.URLParam(r, "id")) if err != nil { h.log.Info("Failed to decode request", zap.String("handler", "deleteAuthorization"), zap.Error(err)) h.api.Err(w, err) return } - if err := h.authSvc.DeleteAuthorization(ctx, req.ID); err != nil { + if err := h.authSvc.DeleteAuthorization(r.Context(), *id); err != nil { // Don't log here, it should already be handled by the service h.api.Err(w, err) return } - h.log.Debug("Auth deleted", zap.String("authID", fmt.Sprint(req.ID))) + h.log.Debug("Auth deleted", zap.String("authID", fmt.Sprint(id))) w.WriteHeader(http.StatusNoContent) } - -type deleteAuthorizationRequest struct { - ID influxdb.ID -} - -// we can clean up and remove these decode functions todo (al) -func decodeDeleteAuthorizationRequest(ctx context.Context, r *http.Request) (*deleteAuthorizationRequest, error) { - id, err := influxdb.IDFromString(chi.URLParam(r, "id")) - if err != nil { - return nil, err - } - - return &deleteAuthorizationRequest{ - ID: *id, - }, nil -} diff --git a/authorization/http_server_test.go b/authorization/http_server_test.go index 4889a933c1..3e1d9d8989 100644 --- a/authorization/http_server_test.go +++ b/authorization/http_server_test.go @@ -1,89 +1,888 @@ -package authorization_test +package authorization import ( + "bytes" "context" + "encoding/json" + "fmt" + "io/ioutil" + "net/http" "net/http/httptest" + "sort" "testing" "github.com/go-chi/chi" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/influxdata/httprouter" "github.com/influxdata/influxdb/v2" - "github.com/influxdata/influxdb/v2/authorization" - ihttp "github.com/influxdata/influxdb/v2/http" + icontext "github.com/influxdata/influxdb/v2/context" "github.com/influxdata/influxdb/v2/inmem" "github.com/influxdata/influxdb/v2/kv" "github.com/influxdata/influxdb/v2/mock" - "github.com/influxdata/influxdb/v2/tenant" itesting "github.com/influxdata/influxdb/v2/testing" "go.uber.org/zap/zaptest" ) -func initAuthorizationService(f itesting.AuthorizationFields, t *testing.T) (influxdb.AuthorizationService, string, func()) { - t.Helper() - - s, stCloser, err := NewTestInmemStore(t) - if err != nil { - t.Fatal(err) - } - - storage, err := authorization.NewStore(s) - if err != nil { - t.Fatal(err) - } - - // set up tenant service - store, err := tenant.NewStore(s) - if err != nil { - t.Fatal(err) - } - ts := tenant.NewService(store) - - ctx := context.Background() - svc := authorization.NewService(storage, ts) - - for _, u := range f.Users { - if err := ts.CreateUser(ctx, u); err != nil { - t.Fatalf("failed to populate users") - } - } - - for _, o := range f.Orgs { - if err := ts.CreateOrganization(ctx, o); err != nil { - t.Fatalf("failed to populate orgs") - } - } - - for _, a := range f.Authorizations { - if err := svc.CreateAuthorization(ctx, a); err != nil { - t.Fatalf("failed to populate authorizations: %v", err) - } - } - - handler := authorization.NewHTTPAuthHandler(zaptest.NewLogger(t), svc, ts, mock.NewLookupService()) - r := chi.NewRouter() - r.Mount(handler.Prefix(), handler) - server := httptest.NewServer(r) - - httpClient, err := ihttp.NewHTTPClient(server.URL, "", false) - if err != nil { - t.Fatal(err) - } - - client := authorization.AuthorizationClientService{ - Client: httpClient, - } - - return &client, "http_authorization", func() { - server.Close() - stCloser() - } -} - func NewTestInmemStore(t *testing.T) (kv.Store, func(), error) { return inmem.NewKVStore(), func() {}, nil } -func TestAuthorizationService(t *testing.T) { - t.Parallel() - // skip FindByToken test here because this function is not supported by the API - itesting.AuthorizationService(initAuthorizationService, t, itesting.WithoutFindByToken()) +func TestService_handlePostAuthorization(t *testing.T) { + type fields struct { + AuthorizationService influxdb.AuthorizationService + TenantService TenantService + LookupService influxdb.LookupService + } + type args struct { + session *influxdb.Authorization + authorization *influxdb.Authorization + } + type wants struct { + statusCode int + contentType string + body string + } + + tests := []struct { + name string + fields fields + args args + wants wants + }{ + { + name: "create a new authorization", + fields: fields{ + AuthorizationService: &mock.AuthorizationService{ + CreateAuthorizationFn: func(ctx context.Context, c *influxdb.Authorization) error { + c.ID = itesting.MustIDBase16("020f755c3c082000") + return nil + }, + }, + TenantService: &tenantService{ + FindUserByIDFn: func(ctx context.Context, id influxdb.ID) (*influxdb.User, error) { + return &influxdb.User{ + ID: id, + Name: "u1", + }, nil + }, + FindOrganizationByIDF: func(ctx context.Context, id influxdb.ID) (*influxdb.Organization, error) { + return &influxdb.Organization{ + ID: id, + Name: "o1", + }, nil + }, + }, + LookupService: &mock.LookupService{ + NameFn: func(ctx context.Context, resource influxdb.ResourceType, id influxdb.ID) (string, error) { + switch resource { + case influxdb.BucketsResourceType: + return "b1", nil + case influxdb.OrgsResourceType: + return "o1", nil + } + return "", fmt.Errorf("bad resource type %s", resource) + }, + }, + }, + args: args{ + session: &influxdb.Authorization{ + Token: "session-token", + ID: itesting.MustIDBase16("020f755c3c082000"), + UserID: itesting.MustIDBase16("aaaaaaaaaaaaaaaa"), + OrgID: itesting.MustIDBase16("020f755c3c083000"), + Description: "can write to authorization resource", + Permissions: []influxdb.Permission{ + { + Action: influxdb.WriteAction, + Resource: influxdb.Resource{ + Type: influxdb.AuthorizationsResourceType, + OrgID: itesting.IDPtr(itesting.MustIDBase16("020f755c3c083000")), + }, + }, + }, + }, + authorization: &influxdb.Authorization{ + ID: itesting.MustIDBase16("020f755c3c082000"), + OrgID: itesting.MustIDBase16("020f755c3c083000"), + Description: "only read dashboards sucka", + Permissions: []influxdb.Permission{ + { + Action: influxdb.ReadAction, + Resource: influxdb.Resource{ + Type: influxdb.DashboardsResourceType, + OrgID: itesting.IDPtr(itesting.MustIDBase16("020f755c3c083000")), + }, + }, + }, + }, + }, + wants: wants{ + statusCode: http.StatusCreated, + contentType: "application/json; charset=utf-8", + body: ` +{ + "createdAt": "0001-01-01T00:00:00Z", + "updatedAt": "0001-01-01T00:00:00Z", + "description": "only read dashboards sucka", + "id": "020f755c3c082000", + "links": { + "self": "/api/v2/authorizations/020f755c3c082000", + "user": "/api/v2/users/aaaaaaaaaaaaaaaa" + }, + "org": "o1", + "orgID": "020f755c3c083000", + "permissions": [ + { + "action": "read", + "resource": { + "type": "dashboards", + "orgID": "020f755c3c083000", + "org": "o1" + } + } + ], + "status": "active", + "token": "new-test-token", + "user": "u1", + "userID": "aaaaaaaaaaaaaaaa" +} +`, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Helper() + + s, _, err := NewTestInmemStore(t) + if err != nil { + t.Fatal(err) + } + + storage, err := NewStore(s) + if err != nil { + t.Fatal(err) + } + + svc := NewService(storage, tt.fields.TenantService) + + handler := NewHTTPAuthHandler(zaptest.NewLogger(t), svc, tt.fields.TenantService, mock.NewLookupService()) + router := chi.NewRouter() + router.Mount(handler.Prefix(), handler) + + req, err := newPostAuthorizationRequest(tt.args.authorization) + if err != nil { + t.Fatalf("failed to create new authorization request: %v", err) + } + b, err := json.Marshal(req) + if err != nil { + t.Fatalf("failed to unmarshal authorization: %v", err) + } + + r := httptest.NewRequest("GET", "http://any.url", bytes.NewReader(b)) + r = r.WithContext(context.WithValue( + context.Background(), + httprouter.ParamsKey, + httprouter.Params{ + { + Key: "userID", + Value: string(tt.args.session.UserID), + }, + })) + + w := httptest.NewRecorder() + + ctx := icontext.SetAuthorizer(context.Background(), tt.args.session) + r = r.WithContext(ctx) + + handler.handlePostAuthorization(w, r) + + res := w.Result() + content := res.Header.Get("Content-Type") + body, _ := ioutil.ReadAll(res.Body) + + if res.StatusCode != tt.wants.statusCode { + t.Logf("headers: %v body: %s", res.Header, body) + t.Errorf("%q. handlePostAuthorization() = %v, want %v", tt.name, res.StatusCode, tt.wants.statusCode) + } + if tt.wants.contentType != "" && content != tt.wants.contentType { + t.Errorf("%q. handlePostAuthorization() = %v, want %v", tt.name, content, tt.wants.contentType) + } + if diff, err := jsonDiff(string(body), tt.wants.body); diff != "" { + t.Errorf("%q. handlePostAuthorization() = ***%s***", tt.name, diff) + } else if err != nil { + t.Errorf("%q, handlePostAuthorization() error: %v", tt.name, err) + } + }) + } +} + +func TestService_handleGetAuthorization(t *testing.T) { + type fields struct { + AuthorizationService influxdb.AuthorizationService + TenantService TenantService + LookupService influxdb.LookupService + } + type args struct { + id string + } + type wants struct { + statusCode int + contentType string + body string + } + + tests := []struct { + name string + fields fields + args args + wants wants + }{ + { + name: "get a authorization by id", + fields: fields{ + AuthorizationService: &mock.AuthorizationService{ + FindAuthorizationByIDFn: func(ctx context.Context, id influxdb.ID) (*influxdb.Authorization, error) { + if id == itesting.MustIDBase16("020f755c3c082000") { + return &influxdb.Authorization{ + ID: itesting.MustIDBase16("020f755c3c082000"), + UserID: itesting.MustIDBase16("020f755c3c082000"), + OrgID: itesting.MustIDBase16("020f755c3c083000"), + Permissions: []influxdb.Permission{ + { + Action: influxdb.ReadAction, + Resource: influxdb.Resource{ + Type: influxdb.BucketsResourceType, + OrgID: itesting.IDPtr(itesting.MustIDBase16("020f755c3c083000")), + ID: func() *influxdb.ID { + id := itesting.MustIDBase16("020f755c3c084000") + return &id + }(), + }, + }, + }, + Token: "hello", + }, nil + } + + return nil, fmt.Errorf("not found") + }, + }, + TenantService: &tenantService{ + FindUserByIDFn: func(ctx context.Context, id influxdb.ID) (*influxdb.User, error) { + return &influxdb.User{ + ID: id, + Name: "u1", + }, nil + }, + FindOrganizationByIDF: func(ctx context.Context, id influxdb.ID) (*influxdb.Organization, error) { + return &influxdb.Organization{ + ID: id, + Name: "o1", + }, nil + }, + }, + LookupService: &mock.LookupService{ + NameFn: func(ctx context.Context, resource influxdb.ResourceType, id influxdb.ID) (string, error) { + switch resource { + case influxdb.BucketsResourceType: + return "b1", nil + case influxdb.OrgsResourceType: + return "o1", nil + } + return "", fmt.Errorf("bad resource type %s", resource) + }, + }, + }, + args: args{ + id: "020f755c3c082000", + }, + wants: wants{ + statusCode: http.StatusOK, + contentType: "application/json; charset=utf-8", + body: ` +{ + "createdAt": "0001-01-01T00:00:00Z", + "updatedAt": "0001-01-01T00:00:00Z", + "description": "", + "id": "020f755c3c082000", + "links": { + "self": "/api/v2/authorizations/020f755c3c082000", + "user": "/api/v2/users/020f755c3c082000" + }, + "org": "o1", + "orgID": "020f755c3c083000", + "permissions": [ + { + "action": "read", + "resource": { + "type": "buckets", + "orgID": "020f755c3c083000", + "id": "020f755c3c084000", + "name": "b1", + "org": "o1" + } + } + ], + "status": "", + "token": "hello", + "user": "u1", + "userID": "020f755c3c082000" +} +`, + }, + }, + { + name: "not found", + fields: fields{ + AuthorizationService: &mock.AuthorizationService{ + FindAuthorizationByIDFn: func(ctx context.Context, id influxdb.ID) (*influxdb.Authorization, error) { + return nil, &influxdb.Error{ + Code: influxdb.ENotFound, + Msg: "authorization not found", + } + }, + }, + TenantService: &tenantService{}, + }, + args: args{ + id: "020f755c3c082000", + }, + wants: wants{ + statusCode: http.StatusNotFound, + body: `{"code":"not found","message":"authorization not found"}`, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Helper() + + handler := NewHTTPAuthHandler(zaptest.NewLogger(t), tt.fields.AuthorizationService, tt.fields.TenantService, mock.NewLookupService()) + router := chi.NewRouter() + router.Mount(handler.Prefix(), handler) + + w := httptest.NewRecorder() + + r := httptest.NewRequest("GET", "http://any.url", nil) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("id", tt.args.id) + r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, rctx)) + + handler.handleGetAuthorization(w, r) + + res := w.Result() + content := res.Header.Get("Content-Type") + body, _ := ioutil.ReadAll(res.Body) + + if res.StatusCode != tt.wants.statusCode { + t.Logf("headers: %v body: %s", res.Header, body) + t.Errorf("%q. handleGetAuthorization() = %v, want %v", tt.name, res.StatusCode, tt.wants.statusCode) + } + if tt.wants.contentType != "" && content != tt.wants.contentType { + t.Errorf("%q. handleGetAuthorization() = %v, want %v", tt.name, content, tt.wants.contentType) + } + if diff, err := jsonDiff(string(body), tt.wants.body); err != nil { + t.Errorf("%q, handleGetAuthorization. error unmarshaling json %v", tt.name, err) + } else if tt.wants.body != "" && diff != "" { + t.Errorf("%q. handleGetAuthorization() = -got/+want %s**", tt.name, diff) + } + }) + } +} + +func TestService_handleGetAuthorizations(t *testing.T) { + type fields struct { + AuthorizationService influxdb.AuthorizationService + TenantService TenantService + } + + type args struct { + queryParams map[string][]string + } + + type wants struct { + statusCode int + contentType string + body string + } + + tests := []struct { + name string + fields fields + args args + wants wants + }{ + { + name: "get all authorizations", + fields: fields{ + &mock.AuthorizationService{ + FindAuthorizationsFn: func(ctx context.Context, filter influxdb.AuthorizationFilter, opts ...influxdb.FindOptions) ([]*influxdb.Authorization, int, error) { + return []*influxdb.Authorization{ + { + ID: itesting.MustIDBase16("0d0a657820696e74"), + Token: "hello", + UserID: itesting.MustIDBase16("2070616e656d2076"), + OrgID: itesting.MustIDBase16("3070616e656d2076"), + Description: "t1", + Permissions: influxdb.OperPermissions(), + }, + { + ID: itesting.MustIDBase16("6669646573207375"), + Token: "example", + UserID: itesting.MustIDBase16("6c7574652c206f6e"), + OrgID: itesting.MustIDBase16("9d70616e656d2076"), + Description: "t2", + Permissions: influxdb.OperPermissions(), + }, + }, 2, nil + }, + }, + &tenantService{ + FindUserByIDFn: func(ctx context.Context, id influxdb.ID) (*influxdb.User, error) { + return &influxdb.User{ + ID: id, + Name: id.String(), + }, nil + }, + + FindOrganizationByIDF: func(ctx context.Context, id influxdb.ID) (*influxdb.Organization, error) { + return &influxdb.Organization{ + ID: id, + Name: id.String(), + }, nil + }, + }, + }, + args: args{}, + wants: wants{ + statusCode: http.StatusOK, + contentType: "application/json; charset=utf-8", + body: fmt.Sprintf(` +{ + "links": { + "self": "/api/v2/authorizations" + }, + "authorizations": [ + { + "links": { + "user": "/api/v2/users/2070616e656d2076", + "self": "/api/v2/authorizations/0d0a657820696e74" + }, + "id": "0d0a657820696e74", + "userID": "2070616e656d2076", + "user": "2070616e656d2076", + "org": "3070616e656d2076", + "orgID": "3070616e656d2076", + "status": "", + "token": "hello", + "description": "t1", + "permissions": %s, + "createdAt": "0001-01-01T00:00:00Z", + "updatedAt": "0001-01-01T00:00:00Z" + }, + { + "links": { + "user": "/api/v2/users/6c7574652c206f6e", + "self": "/api/v2/authorizations/6669646573207375" + }, + "id": "6669646573207375", + "userID": "6c7574652c206f6e", + "user": "6c7574652c206f6e", + "org": "9d70616e656d2076", + "orgID": "9d70616e656d2076", + "status": "", + "token": "example", + "description": "t2", + "permissions": %s, + "createdAt": "0001-01-01T00:00:00Z", + "updatedAt": "0001-01-01T00:00:00Z" + } + ] +} +`, + MustMarshal(influxdb.OperPermissions()), + MustMarshal(influxdb.OperPermissions())), + }, + }, + { + name: "skip authorizations with no org", + fields: fields{ + &mock.AuthorizationService{ + FindAuthorizationsFn: func(ctx context.Context, filter influxdb.AuthorizationFilter, opts ...influxdb.FindOptions) ([]*influxdb.Authorization, int, error) { + return []*influxdb.Authorization{ + { + ID: itesting.MustIDBase16("0d0a657820696e74"), + Token: "hello", + UserID: itesting.MustIDBase16("2070616e656d2076"), + OrgID: itesting.MustIDBase16("3070616e656d2076"), + Description: "t1", + Permissions: influxdb.OperPermissions(), + }, + { + ID: itesting.MustIDBase16("6669646573207375"), + Token: "example", + UserID: itesting.MustIDBase16("6c7574652c206f6e"), + OrgID: itesting.MustIDBase16("9d70616e656d2076"), + Description: "t2", + Permissions: influxdb.OperPermissions(), + }, + }, 2, nil + }, + }, + &tenantService{ + FindUserByIDFn: func(ctx context.Context, id influxdb.ID) (*influxdb.User, error) { + if id.String() == "2070616e656d2076" { + return &influxdb.User{ + ID: id, + Name: id.String(), + }, nil + } + return nil, &influxdb.Error{} + }, + FindOrganizationByIDF: func(ctx context.Context, id influxdb.ID) (*influxdb.Organization, error) { + return &influxdb.Organization{ + ID: id, + Name: id.String(), + }, nil + }, + }, + }, + args: args{}, + wants: wants{ + statusCode: http.StatusOK, + contentType: "application/json; charset=utf-8", + body: fmt.Sprintf(` +{ + "links": { + "self": "/api/v2/authorizations" + }, + "authorizations": [ + { + "links": { + "user": "/api/v2/users/2070616e656d2076", + "self": "/api/v2/authorizations/0d0a657820696e74" + }, + "id": "0d0a657820696e74", + "userID": "2070616e656d2076", + "user": "2070616e656d2076", + "org": "3070616e656d2076", + "orgID": "3070616e656d2076", + "status": "", + "token": "hello", + "description": "t1", + "permissions": %s, + "createdAt": "0001-01-01T00:00:00Z", + "updatedAt": "0001-01-01T00:00:00Z" + } + ] +} +`, + MustMarshal(influxdb.OperPermissions())), + }, + }, + { + name: "skip authorizations with no user", + fields: fields{ + &mock.AuthorizationService{ + FindAuthorizationsFn: func(ctx context.Context, filter influxdb.AuthorizationFilter, opts ...influxdb.FindOptions) ([]*influxdb.Authorization, int, error) { + return []*influxdb.Authorization{ + { + ID: itesting.MustIDBase16("0d0a657820696e74"), + Token: "hello", + UserID: itesting.MustIDBase16("2070616e656d2076"), + OrgID: itesting.MustIDBase16("3070616e656d2076"), + Description: "t1", + Permissions: influxdb.OperPermissions(), + }, + { + ID: itesting.MustIDBase16("6669646573207375"), + Token: "example", + UserID: itesting.MustIDBase16("6c7574652c206f6e"), + OrgID: itesting.MustIDBase16("9d70616e656d2076"), + Description: "t2", + Permissions: influxdb.OperPermissions(), + }, + }, 2, nil + }, + }, + &tenantService{ + FindUserByIDFn: func(ctx context.Context, id influxdb.ID) (*influxdb.User, error) { + return &influxdb.User{ + ID: id, + Name: id.String(), + }, nil + }, + FindOrganizationByIDF: func(ctx context.Context, id influxdb.ID) (*influxdb.Organization, error) { + if id.String() == "3070616e656d2076" { + return &influxdb.Organization{ + ID: id, + Name: id.String(), + }, nil + } + return nil, &influxdb.Error{} + }, + }, + }, + args: args{}, + wants: wants{ + statusCode: http.StatusOK, + contentType: "application/json; charset=utf-8", + body: fmt.Sprintf(` +{ + "links": { + "self": "/api/v2/authorizations" + }, + "authorizations": [ + { + "links": { + "user": "/api/v2/users/2070616e656d2076", + "self": "/api/v2/authorizations/0d0a657820696e74" + }, + "id": "0d0a657820696e74", + "userID": "2070616e656d2076", + "user": "2070616e656d2076", + "org": "3070616e656d2076", + "orgID": "3070616e656d2076", + "status": "", + "token": "hello", + "description": "t1", + "permissions": %s, + "createdAt": "0001-01-01T00:00:00Z", + "updatedAt": "0001-01-01T00:00:00Z" + } + ] +} +`, + MustMarshal(influxdb.OperPermissions())), + }, + }, + { + name: "get all authorizations when there are none", + fields: fields{ + AuthorizationService: &mock.AuthorizationService{ + FindAuthorizationsFn: func(ctx context.Context, filter influxdb.AuthorizationFilter, opts ...influxdb.FindOptions) ([]*influxdb.Authorization, int, error) { + return []*influxdb.Authorization{}, 0, nil + }, + }, + }, + args: args{}, + wants: wants{ + statusCode: http.StatusOK, + contentType: "application/json; charset=utf-8", + body: ` +{ + "links": { + "self": "/api/v2/authorizations" + }, + "authorizations": [] +}`, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Helper() + + s, _, err := NewTestInmemStore(t) + if err != nil { + t.Fatal(err) + } + + storage, err := NewStore(s) + if err != nil { + t.Fatal(err) + } + + svc := NewService(storage, tt.fields.TenantService) + + handler := NewHTTPAuthHandler(zaptest.NewLogger(t), svc, tt.fields.TenantService, mock.NewLookupService()) + router := chi.NewRouter() + router.Mount(handler.Prefix(), handler) + + r := httptest.NewRequest("GET", "http://any.url", nil) + + qp := r.URL.Query() + for k, vs := range tt.args.queryParams { + for _, v := range vs { + qp.Add(k, v) + } + } + r.URL.RawQuery = qp.Encode() + + w := httptest.NewRecorder() + + handler.handleGetAuthorizations(w, r) + + res := w.Result() + content := res.Header.Get("Content-Type") + body, _ := ioutil.ReadAll(res.Body) + + if res.StatusCode != tt.wants.statusCode { + t.Errorf("%q. handleGetAuthorizations() = %v, want %v", tt.name, res.StatusCode, tt.wants.statusCode) + } + if tt.wants.contentType != "" && content != tt.wants.contentType { + t.Errorf("%q. handleGetAuthorizations() = %v, want %v", tt.name, content, tt.wants.contentType) + } + if diff, err := jsonDiff(string(body), tt.wants.body); diff != "" { + t.Errorf("%q. handleGetAuthorizations() = ***%s***", tt.name, diff) + } else if err != nil { + t.Errorf("%q, handleGetAuthorizations() error: %v", tt.name, err) + } + + }) + } +} + +func TestService_handleDeleteAuthorization(t *testing.T) { + type fields struct { + AuthorizationService influxdb.AuthorizationService + TenantService TenantService + } + type args struct { + id string + } + type wants struct { + statusCode int + contentType string + body string + } + + tests := []struct { + name string + fields fields + args args + wants wants + }{ + { + name: "remove a authorization by id", + fields: fields{ + &mock.AuthorizationService{ + DeleteAuthorizationFn: func(ctx context.Context, id influxdb.ID) error { + if id == itesting.MustIDBase16("020f755c3c082000") { + return nil + } + + return fmt.Errorf("wrong id") + }, + }, + &tenantService{}, + }, + args: args{ + id: "020f755c3c082000", + }, + wants: wants{ + statusCode: http.StatusNoContent, + }, + }, + { + name: "authorization not found", + fields: fields{ + &mock.AuthorizationService{ + DeleteAuthorizationFn: func(ctx context.Context, id influxdb.ID) error { + return &influxdb.Error{ + Code: influxdb.ENotFound, + Msg: "authorization not found", + } + }, + }, + &tenantService{}, + }, + args: args{ + id: "020f755c3c082000", + }, + wants: wants{ + statusCode: http.StatusNotFound, + body: `{"code":"not found","message":"authorization not found"}`, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Helper() + + handler := NewHTTPAuthHandler(zaptest.NewLogger(t), tt.fields.AuthorizationService, tt.fields.TenantService, mock.NewLookupService()) + router := chi.NewRouter() + router.Mount(handler.Prefix(), handler) + + w := httptest.NewRecorder() + + r := httptest.NewRequest("GET", "http://any.url", nil) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("id", tt.args.id) + r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, rctx)) + + handler.handleDeleteAuthorization(w, r) + + res := w.Result() + content := res.Header.Get("Content-Type") + body, _ := ioutil.ReadAll(res.Body) + + if res.StatusCode != tt.wants.statusCode { + t.Errorf("%q. handleDeleteAuthorization() = %v, want %v", tt.name, res.StatusCode, tt.wants.statusCode) + } + if tt.wants.contentType != "" && content != tt.wants.contentType { + t.Errorf("%q. handleDeleteAuthorization() = %v, want %v", tt.name, content, tt.wants.contentType) + } + + if tt.wants.body != "" { + if diff, err := jsonDiff(string(body), tt.wants.body); err != nil { + t.Errorf("%q, handleDeleteAuthorization(). error unmarshaling json %v", tt.name, err) + } else if diff != "" { + t.Errorf("%q. handleDeleteAuthorization() = ***%s***", tt.name, diff) + } + } + }) + } +} + +func jsonDiff(s1, s2 string) (diff string, err error) { + if s1 == s2 { + return "", nil + } + + if s1 == "" { + return s2, fmt.Errorf("s1 is empty") + } + + if s2 == "" { + return s1, fmt.Errorf("s2 is empty") + } + + var o1 influxdb.Authorization + if err = json.Unmarshal([]byte(s1), &o1); err != nil { + return + } + + var o2 influxdb.Authorization + if err = json.Unmarshal([]byte(s2), &o2); err != nil { + return + } + + return cmp.Diff(o1, o2, authorizationCmpOptions...), err +} + +var authorizationCmpOptions = cmp.Options{ + cmpopts.EquateEmpty(), + cmpopts.IgnoreFields(influxdb.Authorization{}, "ID", "Token", "CreatedAt", "UpdatedAt"), + cmp.Comparer(func(x, y []byte) bool { + return bytes.Equal(x, y) + }), + cmp.Transformer("Sort", func(in []*influxdb.Authorization) []*influxdb.Authorization { + out := append([]*influxdb.Authorization(nil), in...) // Copy input to avoid mutating it + sort.Slice(out, func(i, j int) bool { + return out[i].ID.String() > out[j].ID.String() + }) + return out + }), +} + +func MustMarshal(o interface{}) []byte { + b, _ := json.Marshal(o) + return b } diff --git a/authorization/middleware_auth.go b/authorization/middleware_auth.go index bd065487be..a6b59ddc2b 100644 --- a/authorization/middleware_auth.go +++ b/authorization/middleware_auth.go @@ -6,17 +6,16 @@ import ( "github.com/influxdata/influxdb/v2" "github.com/influxdata/influxdb/v2/authorizer" - icontext "github.com/influxdata/influxdb/v2/context" ) type AuthedAuthorizationService struct { s influxdb.AuthorizationService - ts influxdb.TenantService + ts TenantService } var _ influxdb.AuthorizationService = (*AuthedAuthorizationService)(nil) -func NewAuthedAuthorizationService(s influxdb.AuthorizationService, ts influxdb.TenantService) *AuthedAuthorizationService { +func NewAuthedAuthorizationService(s influxdb.AuthorizationService, ts TenantService) *AuthedAuthorizationService { return &AuthedAuthorizationService{ s: s, ts: ts, @@ -24,21 +23,6 @@ func NewAuthedAuthorizationService(s influxdb.AuthorizationService, ts influxdb. } func (s *AuthedAuthorizationService) CreateAuthorization(ctx context.Context, a *influxdb.Authorization) error { - if a.UserID == 0 { - auth, err := icontext.GetAuthorizer(ctx) - if err != nil { - return err - } - - user, err := s.ts.FindUserByID(ctx, auth.GetUserID()) - if err != nil { - // if we could not get the user from the Authorization object or the Context, - // then we cannot authorize the user - return err - } - a.UserID = user.ID - } - if _, _, err := authorizer.AuthorizeCreate(ctx, influxdb.AuthorizationsResourceType, a.OrgID); err != nil { return err } diff --git a/authorization/mock_tenant.go b/authorization/mock_tenant.go new file mode 100644 index 0000000000..4054d1a11f --- /dev/null +++ b/authorization/mock_tenant.go @@ -0,0 +1,35 @@ +package authorization + +import ( + "context" + + "github.com/influxdata/influxdb/v2" +) + +// tenantService is a mock implementation of an authorization.tenantService +type tenantService struct { + FindUserByIDFn func(context.Context, influxdb.ID) (*influxdb.User, error) + FindUserFn func(context.Context, influxdb.UserFilter) (*influxdb.User, error) + FindOrganizationByIDF func(ctx context.Context, id influxdb.ID) (*influxdb.Organization, error) + FindOrganizationF func(ctx context.Context, filter influxdb.OrganizationFilter) (*influxdb.Organization, error) +} + +// FindUserByID returns a single User by ID. +func (s *tenantService) FindUserByID(ctx context.Context, id influxdb.ID) (*influxdb.User, error) { + return s.FindUserByIDFn(ctx, id) +} + +// FindUsers returns a list of Users that match filter and the total count of matching Users. +func (s *tenantService) FindUser(ctx context.Context, filter influxdb.UserFilter) (*influxdb.User, error) { + return s.FindUserFn(ctx, filter) +} + +//FindOrganizationByID calls FindOrganizationByIDF. +func (s *tenantService) FindOrganizationByID(ctx context.Context, id influxdb.ID) (*influxdb.Organization, error) { + return s.FindOrganizationByIDF(ctx, id) +} + +//FindOrganization calls FindOrganizationF. +func (s *tenantService) FindOrganization(ctx context.Context, filter influxdb.OrganizationFilter) (*influxdb.Organization, error) { + return s.FindOrganizationF(ctx, filter) +} diff --git a/authorization/service.go b/authorization/service.go index 58a0d3e2e0..5986e9e2d8 100644 --- a/authorization/service.go +++ b/authorization/service.go @@ -14,10 +14,10 @@ var _ influxdb.AuthorizationService = (*Service)(nil) type Service struct { store *Store tokenGenerator influxdb.TokenGenerator - tenantService influxdb.TenantService + tenantService TenantService } -func NewService(st *Store, ts influxdb.TenantService) influxdb.AuthorizationService { +func NewService(st *Store, ts TenantService) influxdb.AuthorizationService { return &Service{ store: st, tokenGenerator: rand.NewTokenGenerator(64), diff --git a/cmd/influxd/launcher/launcher.go b/cmd/influxd/launcher/launcher.go index 531d5fcd5f..2f53296354 100644 --- a/cmd/influxd/launcher/launcher.go +++ b/cmd/influxd/launcher/launcher.go @@ -17,6 +17,7 @@ import ( "github.com/influxdata/flux" platform "github.com/influxdata/influxdb/v2" + "github.com/influxdata/influxdb/v2/authorization" "github.com/influxdata/influxdb/v2/authorizer" "github.com/influxdata/influxdb/v2/bolt" "github.com/influxdata/influxdb/v2/chronograf/server" @@ -957,8 +958,32 @@ func (m *Launcher) run(ctx context.Context) (err error) { onboardHTTPServer = tenant.NewHTTPOnboardHandler(m.log, onboardSvc) } + // feature flagging for new authorization service + var authHTTPServer *kithttp.FeatureHandler { - platformHandler := http.NewPlatformHandler(m.apibackend, http.WithResourceHandler(pkgHTTPServer), http.WithResourceHandler(onboardHTTPServer)) + ts := tenant.NewService(store) // todo (al): remove when tenant is un-flagged + authLogger := m.log.With(zap.String("handler", "authorization")) + + oldBackend := http.NewAuthorizationBackend(authLogger, m.apibackend) + oldBackend.AuthorizationService = authorizer.NewAuthorizationService(authSvc) + oldHandler := http.NewAuthorizationHandler(authLogger, oldBackend) + + authStore, err := authorization.NewStore(m.kvStore) + if err != nil { + m.log.Error("Failed creating new authorization store", zap.Error(err)) + return err + } + authService := authorization.NewService(authStore, ts) + authService = authorization.NewAuthedAuthorizationService(authService, ts) + authService = authorization.NewAuthMetrics(m.reg, authService) + authService = authorization.NewAuthLogger(authLogger, authService) + + newHandler := authorization.NewHTTPAuthHandler(m.log, authService, ts, lookupSvc) + authHTTPServer = kithttp.NewFeatureHandler(feature.NewAuthPackage(), flagger, oldHandler, newHandler, newHandler.Prefix()) + } + + { + platformHandler := http.NewPlatformHandler(m.apibackend, http.WithResourceHandler(pkgHTTPServer), http.WithResourceHandler(onboardHTTPServer), http.WithResourceHandler(authHTTPServer)) httpLogger := m.log.With(zap.String("service", "http")) m.httpServer.Handler = http.NewHandlerFromRegistry( diff --git a/flags.yml b/flags.yml index 6d38252fcf..fc6ff4a11a 100644 --- a/flags.yml +++ b/flags.yml @@ -26,3 +26,10 @@ default: 42 contact: Gavin Cabbage expose: true + +- name: New Auth Package + description: Enables the refactored authorization api + key: newAuth + default: false + contact: Alirie Gray + lifetime: temporary diff --git a/http/api_handler.go b/http/api_handler.go index 76d6bfee0e..bfaaee0a1a 100644 --- a/http/api_handler.go +++ b/http/api_handler.go @@ -127,10 +127,6 @@ func NewAPIHandler(b *APIBackend, opts ...APIHandlerOptFn) *APIHandler { h.Mount("/api/v2", serveLinksHandler(b.HTTPErrorHandler)) - authorizationBackend := NewAuthorizationBackend(b.Logger.With(zap.String("handler", "authorization")), b) - authorizationBackend.AuthorizationService = authorizer.NewAuthorizationService(b.AuthorizationService) - h.Mount(prefixAuthorization, NewAuthorizationHandler(b.Logger, authorizationBackend)) - bucketBackend := NewBucketBackend(b.Logger.With(zap.String("handler", "bucket")), b) bucketBackend.BucketService = authorizer.NewBucketService(b.BucketService, noAuthUserResourceMappingService) h.Mount(prefixBuckets, NewBucketHandler(b.Logger, bucketBackend)) diff --git a/kit/feature/list.go b/kit/feature/list.go index de3da06cfe..b2fd5a6dc8 100644 --- a/kit/feature/list.go +++ b/kit/feature/list.go @@ -30,12 +30,28 @@ func FrontendExample() IntFlag { return frontendExample } +var newAuth = MakeBoolFlag( + "New Auth Package", + "newAuth", + "Alirie Gray", + false, + Temporary, + false, +) + +// NewAuthPackage - Enables the refactored authorization api +func NewAuthPackage() BoolFlag { + return newAuth +} + var all = []Flag{ backendExample, frontendExample, + newAuth, } var byKey = map[string]Flag{ "backendExample": backendExample, "frontendExample": frontendExample, + "newAuth": newAuth, } diff --git a/kit/transport/http/feature_controller.go b/kit/transport/http/feature_controller.go new file mode 100644 index 0000000000..440f8004a5 --- /dev/null +++ b/kit/transport/http/feature_controller.go @@ -0,0 +1,39 @@ +package http + +import ( + "context" + "net/http" + + "github.com/influxdata/influxdb/v2/kit/feature" +) + +// Enabler allows the switching between two HTTP Handlers +type Enabler interface { + Enabled(ctx context.Context, flagger ...feature.Flagger) bool +} + +// FeatureHandler is used to switch requests between an exisiting and a feature flagged +// HTTP Handler on a per-request basis +type FeatureHandler struct { + enabler Enabler + flagger feature.Flagger + oldHandler http.Handler + newHandler http.Handler + prefix string +} + +func NewFeatureHandler(e Enabler, f feature.Flagger, old, new http.Handler, prefix string) *FeatureHandler { + return &FeatureHandler{e, f, old, new, prefix} +} + +func (h *FeatureHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + if h.enabler.Enabled(r.Context(), h.flagger) { + h.newHandler.ServeHTTP(w, r) + return + } + h.oldHandler.ServeHTTP(w, r) +} + +func (h *FeatureHandler) Prefix() string { + return h.prefix +}