diff --git a/pkg/tenant/api/v1/api.go b/pkg/tenant/api/v1/api.go index 31f4f69ac..f603a63bd 100644 --- a/pkg/tenant/api/v1/api.go +++ b/pkg/tenant/api/v1/api.go @@ -50,7 +50,7 @@ type TenantClient interface { TopicList(context.Context, *PageQuery) (*TopicPage, error) TopicDetail(ctx context.Context, id string) (*Topic, error) TopicUpdate(context.Context, *Topic) (*Topic, error) - TopicDelete(ctx context.Context, id string) error + TopicDelete(ctx context.Context, in *Confirmation) (*Confirmation, error) ProjectAPIKeyList(ctx context.Context, id string, in *PageQuery) (*ProjectAPIKeyPage, error) ProjectAPIKeyCreate(ctx context.Context, id string, in *APIKey) (*APIKey, error) @@ -66,6 +66,14 @@ type TenantClient interface { // Top Level Requests and Responses //=========================================================================== +// Confirmation allows APIs to protect users from unintended actions such as deleting +// data by including a confirmation token in the request. +type Confirmation struct { + ID string `json:"id,omitempty"` + Name string `json:"name,omitempty"` + Token string `json:"token,omitempty"` +} + // Reply contains standard fields that are used for generic API responses and errors. type Reply struct { Success bool `json:"success"` @@ -224,6 +232,7 @@ type ProjectTopicPage struct { type Topic struct { ID string `json:"id" uri:"id"` Name string `json:"topic_name"` + State string `json:"state"` Created string `json:"created,omitempty"` Modified string `json:"modified,omitempty"` } diff --git a/pkg/tenant/api/v1/client.go b/pkg/tenant/api/v1/client.go index 7d208522a..f792f6b1a 100644 --- a/pkg/tenant/api/v1/client.go +++ b/pkg/tenant/api/v1/client.go @@ -684,22 +684,22 @@ func (s *APIv1) TopicUpdate(ctx context.Context, in *Topic) (out *Topic, err err return out, nil } -func (s *APIv1) TopicDelete(ctx context.Context, id string) (err error) { - if id == "" { - return ErrTopicIDRequired +func (s *APIv1) TopicDelete(ctx context.Context, in *Confirmation) (out *Confirmation, err error) { + if in.ID == "" { + return nil, ErrTopicIDRequired } - path := fmt.Sprintf("/v1/topics/%s", id) + path := fmt.Sprintf("/v1/topics/%s", in.ID) // Make the HTTP request var req *http.Request - if req, err = s.NewRequest(ctx, http.MethodDelete, path, nil, nil); err != nil { - return err + if req, err = s.NewRequest(ctx, http.MethodDelete, path, in, nil); err != nil { + return nil, err } - if _, err = s.Do(req, nil, true); err != nil { - return err + if _, err = s.Do(req, &out, true); err != nil { + return nil, err } - return nil + return out, nil } func (s *APIv1) ProjectAPIKeyList(ctx context.Context, id string, in *PageQuery) (out *ProjectAPIKeyPage, err error) { diff --git a/pkg/tenant/api/v1/client_test.go b/pkg/tenant/api/v1/client_test.go index bab786901..78896496f 100644 --- a/pkg/tenant/api/v1/client_test.go +++ b/pkg/tenant/api/v1/client_test.go @@ -1073,7 +1073,10 @@ func TestTopicUpdate(t *testing.T) { } func TestTopicDelete(t *testing.T) { - fixture := &api.Reply{} + fixture := &api.Confirmation{ + ID: "topic001", + Token: "token", + } // Creates a test server ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -1090,8 +1093,12 @@ func TestTopicDelete(t *testing.T) { client, err := api.New(ts.URL) require.NoError(t, err, "could not create api client") - err = client.TopicDelete(context.TODO(), "topic001") + req := &api.Confirmation{ + ID: "topic001", + } + out, err := client.TopicDelete(context.TODO(), req) require.NoError(t, err, "could not execute api request") + require.Equal(t, fixture, out, "unexpected response error") } func TestProjectAPIKeyList(t *testing.T) { diff --git a/pkg/tenant/db/tokens.go b/pkg/tenant/db/tokens.go new file mode 100644 index 000000000..4454ce6f4 --- /dev/null +++ b/pkg/tenant/db/tokens.go @@ -0,0 +1,53 @@ +package db + +import ( + "encoding/base64" + "time" + + "github.com/oklog/ulid/v2" + "github.com/rotationalio/ensign/pkg/quarterdeck/keygen" + "github.com/vmihailenco/msgpack/v5" +) + +// NewResourceToken creates a new string token that includes a random secret and +// expires after 5 minutes. +func NewResourceToken(id ulid.ULID) (string, error) { + token := &ResourceToken{ + ID: id, + Secret: keygen.Secret(), + ExpiresAt: time.Now().Add(5 * time.Minute), + } + + return token.create() +} + +// ResourceToken protects access to a resource by encoding an ID with a +// cryptographically secure secret and an expiration time. +type ResourceToken struct { + ID ulid.ULID `msgpack:"id"` + Secret string `msgpack:"token"` + ExpiresAt time.Time `msgpack:"expires_at"` +} + +func (t *ResourceToken) IsExpired() bool { + return t.ExpiresAt.Before(time.Now()) +} + +func (t *ResourceToken) create() (_ string, err error) { + var data []byte + if data, err = msgpack.Marshal(t); err != nil { + return "", err + } + + return base64.RawStdEncoding.EncodeToString(data), nil +} + +// Decode a base64 encoded string into the struct. +func (t *ResourceToken) Decode(token string) (err error) { + var data []byte + if data, err = base64.RawStdEncoding.DecodeString(token); err != nil { + return err + } + + return msgpack.Unmarshal(data, t) +} diff --git a/pkg/tenant/db/tokens_test.go b/pkg/tenant/db/tokens_test.go new file mode 100644 index 000000000..341b8730f --- /dev/null +++ b/pkg/tenant/db/tokens_test.go @@ -0,0 +1,33 @@ +package db_test + +import ( + "testing" + + "github.com/rotationalio/ensign/pkg/tenant/db" + ulids "github.com/rotationalio/ensign/pkg/utils/ulid" + "github.com/stretchr/testify/require" +) + +func TestResourceToken(t *testing.T) { + id := ulids.New() + + // Should be able to create the token from an ID + token, err := db.NewResourceToken(id) + require.NoError(t, err, "could not create token") + + // Should be able to decode the token + decoded := &db.ResourceToken{} + require.NoError(t, decoded.Decode(token), "could not decode token") + require.Equal(t, id, decoded.ID, "decoded ID does not match original ID") + require.False(t, decoded.IsExpired(), "token should not be expired") + require.NotEmpty(t, decoded.Secret, "token secret should not be empty") + + // Tokens should have unique secrets + other, err := db.NewResourceToken(id) + require.NoError(t, err, "could not create token") + require.NotEqual(t, token, other, "tokens should not be equal") + + otherDecoded := &db.ResourceToken{} + require.NoError(t, otherDecoded.Decode(other), "could not decode token") + require.NotEqual(t, decoded.Secret, otherDecoded.Secret, "secrets should not be equal") +} diff --git a/pkg/tenant/db/topics.go b/pkg/tenant/db/topics.go index 464c53676..ac0815a4f 100644 --- a/pkg/tenant/db/topics.go +++ b/pkg/tenant/db/topics.go @@ -5,6 +5,7 @@ import ( "time" "github.com/oklog/ulid/v2" + pb "github.com/rotationalio/ensign/pkg/api/v1beta1" "github.com/rotationalio/ensign/pkg/tenant/api/v1" ulids "github.com/rotationalio/ensign/pkg/utils/ulid" "github.com/vmihailenco/msgpack/v5" @@ -13,12 +14,14 @@ import ( const TopicNamespace = "topics" type Topic struct { - OrgID ulid.ULID `msgpack:"org_id"` - ProjectID ulid.ULID `msgpack:"project_id"` - ID ulid.ULID `msgpack:"id"` - Name string `msgpack:"name"` - Created time.Time `msgpack:"created"` - Modified time.Time `msgpack:"modified"` + OrgID ulid.ULID `msgpack:"org_id"` + ProjectID ulid.ULID `msgpack:"project_id"` + ID ulid.ULID `msgpack:"id"` + Name string `msgpack:"name"` + State pb.TopicTombstone_Status `msgpack:"state"` + ConfirmDeleteToken string `msgpack:"confirm_delete_token"` + Created time.Time `msgpack:"created"` + Modified time.Time `msgpack:"modified"` } var _ Model = &Topic{} diff --git a/pkg/tenant/topics.go b/pkg/tenant/topics.go index dfca75a53..91b643e52 100644 --- a/pkg/tenant/topics.go +++ b/pkg/tenant/topics.go @@ -1,10 +1,14 @@ package tenant import ( + "context" + "errors" "net/http" "github.com/gin-gonic/gin" "github.com/oklog/ulid/v2" + pb "github.com/rotationalio/ensign/pkg/api/v1beta1" + qd "github.com/rotationalio/ensign/pkg/quarterdeck/api/v1" middleware "github.com/rotationalio/ensign/pkg/quarterdeck/middleware" "github.com/rotationalio/ensign/pkg/quarterdeck/tokens" "github.com/rotationalio/ensign/pkg/tenant/api/v1" @@ -257,30 +261,152 @@ func (s *Server) TopicUpdate(c *gin.Context) { c.JSON(http.StatusOK, t.ToAPI()) } -// TopicDelete deletes a topic from a user's request with a given ID -// and returns a 200 OK response instead of an error response. +// TopicDelete completely destroys a topic, removing the metadata in Trtl and as well +// as all of the data in Ensign. Because this is irreversible, the first call returns +// a confirmation token to the user. The user must provide this token in a subsequent +// request in order to confirm the deletion. Because this operation is asynchronous, +// the endpoint returns a 202 Accepted response. // // Route: /topic/:topicID func (s *Server) TopicDelete(c *gin.Context) { var ( - err error + err error + ctx context.Context + claims *tokens.Claims ) + // User credentials are required for Quarterdeck requests + if ctx, err = middleware.ContextFromRequest(c); err != nil { + log.Error().Err(err).Msg("could not create user context from request") + c.JSON(http.StatusUnauthorized, api.ErrorResponse("could not fetch credentials for authenticated user")) + return + } + + // User claims are required to verify that the user owns the topic + if claims, err = middleware.GetClaims(c); err != nil { + log.Error().Err(err).Msg("could not fetch claims from context") + c.JSON(http.StatusUnauthorized, api.ErrorResponse("could not fetch claims from context")) + return + } + // Get the topic ID from the URL and return a 400 response - // if the topic does not exist. + // if the ID is not parseable var topicID ulid.ULID if topicID, err = ulid.Parse(c.Param("topicID")); err != nil { - log.Error().Err(err).Msg("could not parse topic ulid") - c.JSON(http.StatusBadRequest, api.ErrorResponse("could not parse topic ulid")) + log.Warn().Err(err).Msg("could not parse topic id") + c.JSON(http.StatusNotFound, api.ErrorResponse("topic not found")) + return + } + + // Parse the request body for the confirmation token + confirm := &api.Confirmation{} + if err = c.BindJSON(confirm); err != nil { + log.Warn().Err(err).Msg("could not bind topic delete request") + c.JSON(http.StatusBadRequest, api.ErrorResponse("could not bind user request")) + return + } + + // Sanity check that the ID in the request body matches the ID in the URL + if confirm.ID != topicID.String() { + log.Warn().Err(err).Msg("topic id in request body does not match topic id in URL") + c.JSON(http.StatusBadRequest, api.ErrorResponse("id in request body does not match id in URL")) + return + } + + // Fetch the topic metadata from the database + var topic *db.Topic + if topic, err = db.RetrieveTopic(ctx, topicID); err != nil { + if errors.Is(err, db.ErrNotFound) { + log.Warn().Err(err).Str("topicID", topicID.String()).Msg("topic not found") + c.JSON(http.StatusNotFound, api.ErrorResponse("topic not found")) + return + } + log.Error().Err(err).Str("topicID", topicID.String()).Msg("could not retrieve topic") + c.JSON(http.StatusInternalServerError, api.ErrorResponse("could not delete topic")) + return + } + + // Verify that the user owns the topic + if claims.OrgID != topic.OrgID.String() { + log.Warn().Err(err).Str("user_org", claims.OrgID).Str("topic_org", topic.OrgID.String()).Msg("topic OrgID does not match user OrgID") + c.JSON(http.StatusNotFound, api.ErrorResponse("topic not found")) + return + } + + // Send confirmation token if not provided + if confirm.Token == "" { + // Create a short-lived confirmation token in the database + if topic.ConfirmDeleteToken, err = db.NewResourceToken(topic.ID); err != nil { + log.Error().Err(err).Str("topicID", topicID.String()).Msg("could not generate confirmation token") + c.JSON(http.StatusInternalServerError, api.ErrorResponse("could not generate confirmation token")) + return + } + if err = db.UpdateTopic(ctx, topic); err != nil { + log.Error().Err(err).Str("topicID", topicID.String()).Msg("could not save topic") + c.JSON(http.StatusInternalServerError, api.ErrorResponse("could not generate confirmation token")) + return + } + + confirm.Name = topic.Name + confirm.Token = topic.ConfirmDeleteToken + c.JSON(http.StatusOK, confirm) + return + } + + // Verify the confirmation token is valid and not expired + token := &db.ResourceToken{} + if err = token.Decode(confirm.Token); err != nil { + log.Warn().Err(err).Msg("could not decode confirmation token") + c.JSON(http.StatusPreconditionFailed, api.ErrorResponse("invalid confirmation token")) + return + } + + if token.IsExpired() { + log.Warn().Msg("confirmation token has expired") + c.JSON(http.StatusPreconditionFailed, api.ErrorResponse("invalid confirmation token")) + return + } + + // Verify that we have the right token + if token.ID.Compare(topic.ID) != 0 { + log.Warn().Err(err).Str("token_id", token.ID.String()).Str("topic_id", topic.ID.String()).Msg("confirmation token does not match topic") + c.JSON(http.StatusPreconditionFailed, api.ErrorResponse("invalid confirmation token")) + return + } + + // Request access to the project from Quarterdeck + req := &qd.Project{ + ProjectID: topic.ProjectID, + } + var rep *qd.LoginReply + if rep, err = s.quarterdeck.ProjectAccess(ctx, req); err != nil { + log.Error().Err(err).Msg("could not request one-time claims") + c.JSON(qd.ErrorStatus(err), api.ErrorResponse("could not delete topic")) + return + } + + // Create the Ensign context from the one-time claims + ensignContext := qd.ContextWithToken(ctx, rep.AccessToken) + + // Send the delete topic request to Ensign + deleteRequest := &pb.TopicMod{ + Id: topic.ID.String(), + Operation: pb.TopicMod_DESTROY, + } + var tombstone *pb.TopicTombstone + if _, err = s.ensign.DeleteTopic(ensignContext, deleteRequest); err != nil { + log.Error().Err(err).Msg("could not delete topic in ensign") + c.JSON(qd.ErrorStatus(err), api.ErrorResponse("could not delete topic")) return } - // Delete the topic and return a 404 response if it cannot be removed. - if err = db.DeleteTopic(c.Request.Context(), topicID); err != nil { - log.Error().Err(err).Str("topicID", topicID.String()).Msg("could not delete topic") - c.JSON(http.StatusNotFound, api.ErrorResponse("could not delete topic")) + // The delete request is asynchronous so just update the state in the database + topic.State = tombstone.State + if err = db.UpdateTopic(ctx, topic); err != nil { + log.Error().Err(err).Str("topicID", topicID.String()).Msg("could not update topic state") + c.JSON(http.StatusInternalServerError, api.ErrorResponse("could not delete topic")) return } - c.Status(http.StatusOK) + c.Status(http.StatusAccepted) } diff --git a/pkg/tenant/topics_test.go b/pkg/tenant/topics_test.go index 379b051ac..3e2529d56 100644 --- a/pkg/tenant/topics_test.go +++ b/pkg/tenant/topics_test.go @@ -453,12 +453,35 @@ func (suite *tenantTestSuite) TestTopicDelete() { require := suite.Require() ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) topicID := "01GNA926JCTKDH3VZBTJM8MAF6" + orgID := "01GNA91N6WMCWNG9MVSK47ZS88" + projectID := "02ABC91N6WMCWNG9MVSK47ZSYZ" defer cancel() // Connect to mock trtl database. trtl := db.GetMock() defer trtl.Reset() + topic := &db.Topic{ + OrgID: ulid.MustParse(orgID), + ProjectID: ulid.MustParse(projectID), + ID: ulid.MustParse(topicID), + Name: "mytopic", + } + data, err := topic.MarshalValue() + require.NoError(err, "could not marshal topic data") + + // Configure Trtl to return the fixture on Get requests. + trtl.OnGet = func(ctx context.Context, gr *pb.GetRequest) (*pb.GetReply, error) { + return &pb.GetReply{ + Value: data, + }, nil + } + + // Configure Trtl to return a success response on Put requests. + trtl.OnPut = func(ctx context.Context, pr *pb.PutRequest) (*pb.PutReply, error) { + return &pb.PutReply{}, nil + } + // Call OnDelete method and return a DeleteReply. trtl.OnDelete = func(ctx context.Context, dr *pb.DeleteRequest) (*pb.DeleteReply, error) { return &pb.DeleteReply{}, nil @@ -473,12 +496,15 @@ func (suite *tenantTestSuite) TestTopicDelete() { // Endpoint must be authenticated require.NoError(suite.SetClientCSRFProtection(), "could not set client csrf protection") - err := suite.client.TopicDelete(ctx, "01GNA926JCTKDH3VZBTJM8MAF6") + req := &api.Confirmation{ + ID: topicID, + } + _, err = suite.client.TopicDelete(ctx, req) suite.requireError(err, http.StatusUnauthorized, "this endpoint requires authentication", "expected error when not authenticated") // User must have the correct permissions require.NoError(suite.SetClientCredentials(claims), "could not set client credentials") - err = suite.client.TopicDelete(ctx, "01GNA926JCTKDH3VZBTJM8MAF6") + _, err = suite.client.TopicDelete(ctx, req) suite.requireError(err, http.StatusUnauthorized, "user does not have permission to perform this operation", "expected error when user does not have permission") // Set valid permissions for the rest of the tests @@ -486,17 +512,29 @@ func (suite *tenantTestSuite) TestTopicDelete() { require.NoError(suite.SetClientCredentials(claims), "could not set client credentials") // Should return an error if the topic does not exist. - err = suite.client.TopicDelete(ctx, "invalid") - suite.requireError(err, http.StatusBadRequest, "could not parse topic ulid", "expected error when topic does not exist") + req.ID = "invalid" + _, err = suite.client.TopicDelete(ctx, req) + suite.requireError(err, http.StatusNotFound, "topic not found", "expected error when topic does not exist") + + // Should return an error if the orgIDs don't match + req.ID = topicID + _, err = suite.client.TopicDelete(ctx, req) + suite.requireError(err, http.StatusNotFound, "topic not found", "expected error when orgIDs don't match") - err = suite.client.TopicDelete(ctx, topicID) + // Retrieve a confirmation from the first successful request. + claims.OrgID = orgID + require.NoError(suite.SetClientCredentials(claims), "could not set client credentials") + reply, err := suite.client.TopicDelete(ctx, req) require.NoError(err, "could not delete topic") + require.Equal(reply.ID, topicID, "expected topic ID to match") + require.Equal(reply.Name, topic.Name, "expected topic name to match") + require.NotEmpty(reply.Token, "expected confirmation token to be set") // Should return an error if the topic ID is parsed but not found. - trtl.OnDelete = func(ctx context.Context, dr *pb.DeleteRequest) (*pb.DeleteReply, error) { - return nil, errors.New("key not found") + trtl.OnGet = func(ctx context.Context, gr *pb.GetRequest) (*pb.GetReply, error) { + return nil, status.Error(codes.NotFound, "key not found") } - err = suite.client.TopicDelete(ctx, "01GNA926JCTKDH3VZBTJM8MAF6") - suite.requireError(err, http.StatusNotFound, "could not delete topic", "expected error when topic ID is not found") + _, err = suite.client.TopicDelete(ctx, req) + suite.requireError(err, http.StatusNotFound, "topic not found", "expected error when topic ID is not found") }