diff --git a/api.go b/api.go index ceac4c04..8d46681b 100644 --- a/api.go +++ b/api.go @@ -40,6 +40,7 @@ func APICommand() *cobra.Command { WithResourceAndHandler(&billingv1.BillingEntity{}, ob.Build). WithResourceAndHandler(&userv1.Invitation{}, ib.Build). WithResourceAndHandler(secretstorage.NewStatusSubResourceRegisterer(&userv1.Invitation{}), ib.Build). + WithResourceAndHandler(&userv1.InvitationRedeemRequest{}, ib.BuildRedeem). WithoutEtcd(). ExposeLoopbackAuthorizer(). ExposeLoopbackMasterClientConfig(). @@ -102,7 +103,11 @@ type invitationStorageBuilder struct { } func (i *invitationStorageBuilder) Build(s *runtime.Scheme, g genericregistry.RESTOptionsGetter) (rest.Storage, error) { - return user.NewInvitationStorage(i.backingNS, *i.usernamePrefix)(s, g) + return user.NewInvitationStorage(i.backingNS)(s, g) +} + +func (i *invitationStorageBuilder) BuildRedeem(s *runtime.Scheme, g genericregistry.RESTOptionsGetter) (rest.Storage, error) { + return user.NewInvitationRedeemStorage(*i.usernamePrefix)(s, g) } type organizationStatusRegisterer struct { diff --git a/apis/user/v1/invitation_types.go b/apis/user/v1/invitation_types.go index fd308251..fa9ef096 100644 --- a/apis/user/v1/invitation_types.go +++ b/apis/user/v1/invitation_types.go @@ -166,6 +166,70 @@ func (o *RedeemOptions) ConvertFromUrlValues(values *url.Values) error { return convert_url_Values_To__RedeemOptions(values, o) } +var _ resource.Object = &InvitationRedeemRequest{} + +// +kubebuilder:object:root=true +// InvitationRedeemRequest is a request to redeem an invitation +type InvitationRedeemRequest struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + // Token is the token to redeem the invitation + Token string `json:"token"` +} + +// GetObjectMeta returns the objects meta reference. +func (o *InvitationRedeemRequest) GetObjectMeta() *metav1.ObjectMeta { + return &o.ObjectMeta +} + +// GetGroupVersionResource returns the GroupVersionResource for this resource. +// The resource should be the all lowercase and pluralized kind +func (o *InvitationRedeemRequest) GetGroupVersionResource() schema.GroupVersionResource { + return schema.GroupVersionResource{ + Group: GroupVersion.Group, + Version: GroupVersion.Version, + Resource: "invitationredeemrequests", + } +} + +// IsStorageVersion returns true if the object is also the internal version -- i.e. is the type defined for the API group or an alias to this object. +// If false, the resource is expected to implement MultiVersionObject interface. +func (o *InvitationRedeemRequest) IsStorageVersion() bool { + return true +} + +// NamespaceScoped returns true if the object is namespaced +func (o *InvitationRedeemRequest) NamespaceScoped() bool { + return false +} + +// New returns a new instance of the resource +func (o *InvitationRedeemRequest) New() runtime.Object { + return &InvitationRedeemRequest{} +} + +// NewList return a new list instance of the resource +func (o *InvitationRedeemRequest) NewList() runtime.Object { + return &InvitationRedeemRequestList{} +} + +var _ resource.ObjectList = &InvitationRedeemRequestList{} + +// +kubebuilder:object:root=true +// InvitationList contains a list of Invitations +type InvitationRedeemRequestList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + + Items []InvitationRedeemRequest `json:"items"` +} + +// GetListMeta returns the list meta reference. +func (in *InvitationRedeemRequestList) GetListMeta() *metav1.ListMeta { + return &in.ListMeta +} + func init() { - SchemeBuilder.Register(&Invitation{}, &InvitationList{}) + SchemeBuilder.Register(&Invitation{}, &InvitationList{}, &InvitationRedeemRequest{}) } diff --git a/apis/user/v1/zz_generated.deepcopy.go b/apis/user/v1/zz_generated.deepcopy.go index bdf18b18..9512a851 100644 --- a/apis/user/v1/zz_generated.deepcopy.go +++ b/apis/user/v1/zz_generated.deepcopy.go @@ -69,6 +69,63 @@ func (in *InvitationList) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *InvitationRedeemRequest) DeepCopyInto(out *InvitationRedeemRequest) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new InvitationRedeemRequest. +func (in *InvitationRedeemRequest) DeepCopy() *InvitationRedeemRequest { + if in == nil { + return nil + } + out := new(InvitationRedeemRequest) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *InvitationRedeemRequest) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *InvitationRedeemRequestList) DeepCopyInto(out *InvitationRedeemRequestList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]InvitationRedeemRequest, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new InvitationRedeemRequestList. +func (in *InvitationRedeemRequestList) DeepCopy() *InvitationRedeemRequestList { + if in == nil { + return nil + } + out := new(InvitationRedeemRequestList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *InvitationRedeemRequestList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *InvitationSpec) DeepCopyInto(out *InvitationSpec) { *out = *in diff --git a/apiserver/user/invitation_redeem.go b/apiserver/user/invitation_redeem.go index bd782ccf..31a74dbd 100644 --- a/apiserver/user/invitation_redeem.go +++ b/apiserver/user/invitation_redeem.go @@ -7,6 +7,7 @@ import ( "strings" "time" + apierrors "k8s.io/apimachinery/pkg/api/errors" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -17,7 +18,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" userv1 "github.com/appuio/control-api/apis/user/v1" - "github.com/appuio/control-api/apiserver/secretstorage" ) //+kubebuilder:rbac:groups="rbac.appuio.io",resources=invitations,verbs=get;list;watch @@ -25,88 +25,95 @@ import ( //+kubebuilder:rbac:groups="rbac.appuio.io",resources=invitations/status,verbs=get;update;patch //+kubebuilder:rbac:groups="user.appuio.io",resources=invitations/status,verbs=get;update;patch -var _ rest.Connecter = &invitationRedeemer{} -var _ rest.StandardStorage = &invitationRedeemer{} +var _ rest.Creater = &invitationRedeemer{} +var _ rest.Storage = &invitationRedeemer{} var _ rest.Scoper = &invitationRedeemer{} type invitationRedeemer struct { - secretstorage.ScopedStandardStorage client client.Client usernamePrefix string } -func (ir *invitationRedeemer) ConnectMethods() []string { - return []string{"REDEEM"} +func (ir invitationRedeemer) NamespaceScoped() bool { + return false } -func (ir *invitationRedeemer) NewConnectOptions() (runtime.Object, bool, string) { - return &userv1.RedeemOptions{}, false, "" +func (ir invitationRedeemer) New() runtime.Object { + return &userv1.InvitationRedeemRequest{} } -// Connect implements the REDEEM method for invitations. -// It is used to redeem an invitation by a user. +func (ir invitationRedeemer) Destroy() {} + +// Create implements redeeming invitations, it accepts `InvitationRedeemRequest`. // The user is identified by the username in the request context. -// The token is taken from the path. // If the invitation is valid, the invitation is marked as redeemed, the user, and a snapshot of the invitations's targets are stored in the status. // The snapshot is later used in a controller to add the user to the targets in an idempotent and retryable way. // If user or token are invalid, the request is rejected with a 403. -func (s *invitationRedeemer) Connect(ctx context.Context, name string, options runtime.Object, responder rest.Responder) (http.Handler, error) { - l := klog.FromContext(ctx).WithName("InvitationRedeemer.Connect").WithValues("invitation", name) - opts := options.(*userv1.RedeemOptions) - // Might come from the path, so we need to trim the leading slash - token := strings.TrimLeft(opts.Token, "/") - - return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - inv := &userv1.Invitation{} - if err := s.client.Get(ctx, client.ObjectKey{Name: name}, inv); err != nil { - responder.Error(err) - return - } +func (s *invitationRedeemer) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, opts *metav1.CreateOptions) (runtime.Object, error) { + irr, ok := obj.(*userv1.InvitationRedeemRequest) + if !ok { + return nil, fmt.Errorf("not an InvitationRedeemRequest: %#v", obj) + } - tokenValid := inv.Status.Token != "" && inv.Status.ValidUntil.After(time.Now()) - if inv.IsRedeemed() || !tokenValid || inv.Status.Token != token { - l.Info("invalid token") - forbidden(responder) - return - } + name := irr.Name + token := irr.Token - user, ok := userFrom(ctx, s.usernamePrefix) - if !ok { - l.Info("no allowed user found in request context", "usernamePrefix", s.usernamePrefix) - forbidden(responder) - return - } + l := klog.FromContext(ctx).WithName("InvitationRedeemer.Create").WithValues("invitation", name) - ts := make([]userv1.TargetStatus, len(inv.Spec.TargetRefs)) - for i, target := range inv.Spec.TargetRefs { - ts[i] = userv1.TargetStatus{ - TargetRef: target, - Condition: metav1.Condition{ - Type: userv1.ConditionRedeemed, - Status: metav1.ConditionUnknown, - }, - } - } + inv := &userv1.Invitation{} + if err := s.client.Get(ctx, client.ObjectKey{Name: name}, inv); err != nil { + return nil, fmt.Errorf("failed to get invitation: %w", err) + } + + if inv.Status.Token == "" { + l.Info("token is empty") + return nil, errForbidden() + } + if !inv.Status.ValidUntil.After(time.Now()) { + l.Info("invitation is expired") + return nil, errForbidden() + } + if inv.IsRedeemed() { + l.Info("invitation is already redeemed") + return nil, errForbidden() + } + if inv.Status.Token != token { + l.Info("token does not match") + return nil, errForbidden() + } + + user, ok := userFrom(ctx, s.usernamePrefix) + if !ok { + l.Info("no allowed user found in request context", "usernamePrefix", s.usernamePrefix) + return nil, errForbidden() + } - inv.Status.TargetStatuses = ts - inv.Status.RedeemedBy = user.GetName() - apimeta.SetStatusCondition(&inv.Status.Conditions, metav1.Condition{ - Type: userv1.ConditionRedeemed, - Status: metav1.ConditionTrue, - Reason: userv1.ConditionRedeemed, - Message: fmt.Sprintf("Redeemed by %q", user.GetName()), - }) - - if err := s.client.Status().Update(ctx, inv); err != nil { - responder.Error(err) - return + ts := make([]userv1.TargetStatus, len(inv.Spec.TargetRefs)) + for i, target := range inv.Spec.TargetRefs { + ts[i] = userv1.TargetStatus{ + TargetRef: target, + Condition: metav1.Condition{ + Type: userv1.ConditionRedeemed, + Status: metav1.ConditionUnknown, + }, } + } - responder.Object(http.StatusOK, &metav1.Status{ - Status: metav1.StatusSuccess, - }) - }), nil + inv.Status.TargetStatuses = ts + inv.Status.RedeemedBy = user.GetName() + apimeta.SetStatusCondition(&inv.Status.Conditions, metav1.Condition{ + Type: userv1.ConditionRedeemed, + Status: metav1.ConditionTrue, + Reason: userv1.ConditionRedeemed, + Message: fmt.Sprintf("Redeemed by %q", user.GetName()), + }) + + if err := s.client.Status().Update(ctx, inv); err != nil { + return nil, fmt.Errorf("failed to update invitation: %w", err) + } + + return irr, nil } // userFrom returns the user from the context if it is a non-serviceaccount user and has the usernamePrefix. @@ -128,10 +135,11 @@ func userFrom(ctx context.Context, usernamePrefix string) (u user.Info, ok bool) return user, true } -func forbidden(resp rest.Responder) { - resp.Object(http.StatusForbidden, &metav1.Status{ - Status: metav1.StatusFailure, - Code: http.StatusForbidden, - Reason: metav1.StatusReasonUnauthorized, - }) +func errForbidden() *apierrors.StatusError { + return &apierrors.StatusError{ + ErrStatus: metav1.Status{ + Status: metav1.StatusFailure, + Code: http.StatusForbidden, + Reason: metav1.StatusReasonForbidden, + }} } diff --git a/apiserver/user/invitation_redeem_test.go b/apiserver/user/invitation_redeem_test.go index cff6b2b1..278aec57 100644 --- a/apiserver/user/invitation_redeem_test.go +++ b/apiserver/user/invitation_redeem_test.go @@ -3,13 +3,13 @@ package user import ( "context" "net/http" - "net/http/httptest" "testing" "time" - "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + apierrors "k8s.io/apimachinery/pkg/api/errors" + apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/authentication/user" @@ -19,10 +19,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" userv1 "github.com/appuio/control-api/apis/user/v1" - "github.com/appuio/control-api/apiserver/authwrapper/mock" ) -func TestConnect_Redeem_Success(t *testing.T) { +func TestCreate_Redeem_Success(t *testing.T) { target := userv1.TargetRef{ APIGroup: "appuio.io", Kind: "Team", @@ -36,7 +35,7 @@ func TestConnect_Redeem_Success(t *testing.T) { c := prepareTest(t, inv) subject := invitationRedeemer{client: c} - executeRequest(t, subject, "redeeming-user", "/"+inv.Status.Token, http.StatusOK) + executeRequest(t, subject, "redeeming-user", inv.Status.Token, http.StatusOK) require.NoError(t, c.Get(context.Background(), client.ObjectKeyFromObject(inv), inv)) assert.True(t, inv.IsRedeemed()) @@ -56,7 +55,7 @@ func TestConnect_Redeem_Fail_InvalidToken(t *testing.T) { c := prepareTest(t, redeemableInvitation()) subject := invitationRedeemer{client: c} - executeRequest(t, subject, "redeeming-user", "/invalid", http.StatusForbidden) + executeRequest(t, subject, "redeeming-user", "invalid", http.StatusForbidden) } func TestConnect_Redeem_Fail_InvalidUser(t *testing.T) { @@ -66,7 +65,39 @@ func TestConnect_Redeem_Fail_InvalidUser(t *testing.T) { usernamePrefix: "appuio#", } - executeRequest(t, subject, "redeeming-user", "/token", http.StatusForbidden) + executeRequest(t, subject, "redeeming-user", "token", http.StatusForbidden) +} + +func TestConnect_Redeem_Fail_Expired(t *testing.T) { + inv := redeemableInvitation() + inv.Status.ValidUntil = metav1.NewTime(metav1.Now().Add(-time.Hour)) + c := prepareTest(t, inv) + subject := invitationRedeemer{client: c} + + executeRequest(t, subject, "redeeming-user", "token", http.StatusForbidden) +} + +// The token is added asynchronously, so it might not be available yet. +func TestConnect_Redeem_Fail_NoTokenYet(t *testing.T) { + inv := redeemableInvitation() + inv.Status.Token = "" + inv.Status.ValidUntil = metav1.Time{} + c := prepareTest(t, inv) + subject := invitationRedeemer{client: c} + + executeRequest(t, subject, "redeeming-user", "token", http.StatusForbidden) +} + +func TestConnect_Redeem_Fail_AlreadyRedeemed(t *testing.T) { + inv := redeemableInvitation() + apimeta.SetStatusCondition(&inv.Status.Conditions, metav1.Condition{ + Type: userv1.ConditionRedeemed, + Status: metav1.ConditionTrue, + }) + c := prepareTest(t, inv) + subject := invitationRedeemer{client: c} + + executeRequest(t, subject, "redeeming-user", "token", http.StatusForbidden) } func redeemableInvitation() *userv1.Invitation { @@ -84,22 +115,27 @@ func redeemableInvitation() *userv1.Invitation { func executeRequest(t *testing.T, subject invitationRedeemer, username, token string, expectedHTTPStatus int) { t.Helper() - ctrl := gomock.NewController(t) - r := mock.NewMockResponder(ctrl) - defer ctrl.Finish() - r.EXPECT().Object(expectedHTTPStatus, gomock.Any()) - reqCtx := request.WithUser(context.Background(), &user.DefaultInfo{ Name: username, }) - h, err := subject.Connect(reqCtx, "subject", &userv1.RedeemOptions{ - Token: "/" + token, - }, r) - require.NoError(t, err) - require.NotNil(t, h) - req, err := http.NewRequestWithContext(reqCtx, "REDEEM", "", nil) - require.NoError(t, err) - h.ServeHTTP(httptest.NewRecorder(), req) + h, err := subject.Create(reqCtx, &userv1.InvitationRedeemRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: "subject", + }, + Token: token, + }, nil, &metav1.CreateOptions{}) + + if expectedHTTPStatus == http.StatusOK { + require.NotNil(t, h) + require.NoError(t, err) + return + } + + require.Error(t, err) + if assert.IsType(t, &apierrors.StatusError{}, err) { + status := err.(*apierrors.StatusError) + assert.Equal(t, expectedHTTPStatus, int(status.Status().Code)) + } } func prepareTest(t *testing.T, initObjs ...client.Object) client.WithWatch { diff --git a/apiserver/user/invitation_storage.go b/apiserver/user/invitation_storage.go index 48be32ec..503ec1a8 100644 --- a/apiserver/user/invitation_storage.go +++ b/apiserver/user/invitation_storage.go @@ -15,22 +15,31 @@ import ( "github.com/appuio/control-api/apiserver/secretstorage" ) -// New returns a new storage provider with RBAC authentication for BillingEntities -func NewInvitationStorage(backingNS, usernamePrefix string) restbuilder.ResourceHandlerProvider { +// NewInvitationRedeemStorage creates a new REST storage for InvitationRedeemRequest objects +func NewInvitationRedeemStorage(usernamePrefix string) restbuilder.ResourceHandlerProvider { return func(s *runtime.Scheme, g genericregistry.RESTOptionsGetter) (rest.Storage, error) { - c, err := client.NewWithWatch(loopback.GetLoopbackMasterClientConfig(), client.Options{}) + c, err := buildClient() if err != nil { return nil, err } - err = userv1.AddToScheme(c.Scheme()) - if err != nil { - return nil, err + stor := &invitationRedeemer{ + client: c, + usernamePrefix: usernamePrefix, } - err = rbacv1.AddToScheme(c.Scheme()) + + return stor, nil + } +} + +// NewInvitationStorage returns a new storage provider with RBAC authentication for BillingEntities +func NewInvitationStorage(backingNS string) restbuilder.ResourceHandlerProvider { + return func(s *runtime.Scheme, g genericregistry.RESTOptionsGetter) (rest.Storage, error) { + c, err := buildClient() if err != nil { return nil, err } + stor, err := secretstorage.NewStorage(&userv1.Invitation{}, c, backingNS) if err != nil { return nil, err @@ -41,13 +50,6 @@ func NewInvitationStorage(backingNS, usernamePrefix string) restbuilder.Resource client: c, } - // Warning: Should be the last storage layer before authorization since it expands the interface with the Connecter interface - stor = &invitationRedeemer{ - ScopedStandardStorage: stor, - client: c, - usernamePrefix: usernamePrefix, - } - astor, err := authwrapper.NewAuthorizedStorage(stor, metav1.GroupVersionResource{ Group: "rbac.appuio.io", Version: "v1", @@ -60,3 +62,19 @@ func NewInvitationStorage(backingNS, usernamePrefix string) restbuilder.Resource return astor, nil } } + +func buildClient() (client.WithWatch, error) { + c, err := client.NewWithWatch(loopback.GetLoopbackMasterClientConfig(), client.Options{}) + if err != nil { + return nil, err + } + + if err = userv1.AddToScheme(c.Scheme()); err != nil { + return nil, err + } + if err = rbacv1.AddToScheme(c.Scheme()); err != nil { + return nil, err + } + + return c, nil +} diff --git a/config/user-rbac/basic-user-role.yml b/config/user-rbac/basic-user-role.yml index c1e6e338..16a94133 100644 --- a/config/user-rbac/basic-user-role.yml +++ b/config/user-rbac/basic-user-role.yml @@ -24,8 +24,11 @@ rules: # `get` permissions are created when creating a new BillingEntity - apiGroups: ["rbac.appuio.io"] resources: ["invitations"] - verbs: ["watch", "list", "redeem"] + verbs: ["watch", "list"] - apiGroups: ["user.appuio.io"] resources: ["invitations"] - # empty verb to allow upstream server to pass custom verbs to our aggregated API server - verbs: ["get", "watch", "list", "redeem", ""] + verbs: ["get", "watch", "list"] +# Allow redeeming invitations +- apiGroups: ["user.appuio.io"] + resources: ["invitationredeemrequests"] + verbs: ["create"]