Remove LoginRequestStatus.User, for now
As discussed in API review, this field exists for convenience right now. Since the username/groups are encoded in the Credential sent in the LoginRequestStatus, the client still has access to their user/groups information. We want to remove this for now to be conservative and limit our API surface area (smaller surface area = less to maintain). We can always add this back in the future. Signed-off-by: Andrew Keesler <akeesler@vmware.com>
This commit is contained in:
parent
c6f1defa9d
commit
dd8ce677ba
@ -114,10 +114,6 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
|
||||
ClientCertificateData: string(certPEM),
|
||||
ClientKeyData: string(keyPEM),
|
||||
},
|
||||
User: &placeholderapi.User{
|
||||
Name: username,
|
||||
Groups: groups,
|
||||
},
|
||||
},
|
||||
}, nil
|
||||
}
|
||||
|
@ -114,10 +114,6 @@ func TestCreate(t *testing.T) {
|
||||
|
||||
r.Equal(response, &placeholderapi.LoginRequest{
|
||||
Status: placeholderapi.LoginRequestStatus{
|
||||
User: &placeholderapi.User{
|
||||
Name: "test-user",
|
||||
Groups: []string{"test-group-1", "test-group-2"},
|
||||
},
|
||||
Credential: &placeholderapi.LoginRequestCredential{
|
||||
ExpirationTimestamp: metav1.Time{},
|
||||
ClientCertificateData: "test-cert",
|
||||
|
@ -40,23 +40,11 @@ type LoginRequestCredential struct {
|
||||
ClientKeyData string
|
||||
}
|
||||
|
||||
type User struct {
|
||||
// Identity Provider name for authenticated user.
|
||||
Name string
|
||||
|
||||
// Identity Provider groups for authenticated user.
|
||||
Groups []string
|
||||
}
|
||||
|
||||
type LoginRequestStatus struct {
|
||||
// A Credential will be returned for a successful login request.
|
||||
// +optional
|
||||
Credential *LoginRequestCredential
|
||||
|
||||
// A User will be populated from the Identity Provider.
|
||||
// +optional
|
||||
User *User
|
||||
|
||||
// An error message will be returned for an unsuccessful login request.
|
||||
// +optional
|
||||
Message *string
|
||||
|
@ -40,23 +40,11 @@ type LoginRequestCredential struct {
|
||||
ClientKeyData string `json:"clientKeyData,omitempty"`
|
||||
}
|
||||
|
||||
type User struct {
|
||||
// Identity Provider name for authenticated user.
|
||||
Name string `json:"name,omitempty"`
|
||||
|
||||
// Identity Provider groups for authenticated user.
|
||||
Groups []string `json:"groups"`
|
||||
}
|
||||
|
||||
type LoginRequestStatus struct {
|
||||
// A Credential will be returned for a successful login request.
|
||||
// +optional
|
||||
Credential *LoginRequestCredential `json:"credential,omitempty"`
|
||||
|
||||
// A User will be populated from the Identity Provider.
|
||||
// +optional
|
||||
User *User `json:"user,omitempty"`
|
||||
|
||||
// An error message will be returned for an unsuccessful login request.
|
||||
// +optional
|
||||
Message *string `json:"message,omitempty"`
|
||||
|
@ -84,16 +84,6 @@ func RegisterConversions(s *runtime.Scheme) error {
|
||||
}); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := s.AddGeneratedConversionFunc((*User)(nil), (*placeholder.User)(nil), func(a, b interface{}, scope conversion.Scope) error {
|
||||
return Convert_v1alpha1_User_To_placeholder_User(a.(*User), b.(*placeholder.User), scope)
|
||||
}); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := s.AddGeneratedConversionFunc((*placeholder.User)(nil), (*User)(nil), func(a, b interface{}, scope conversion.Scope) error {
|
||||
return Convert_placeholder_User_To_v1alpha1_User(a.(*placeholder.User), b.(*User), scope)
|
||||
}); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@ -201,7 +191,6 @@ func Convert_placeholder_LoginRequestSpec_To_v1alpha1_LoginRequestSpec(in *place
|
||||
|
||||
func autoConvert_v1alpha1_LoginRequestStatus_To_placeholder_LoginRequestStatus(in *LoginRequestStatus, out *placeholder.LoginRequestStatus, s conversion.Scope) error {
|
||||
out.Credential = (*placeholder.LoginRequestCredential)(unsafe.Pointer(in.Credential))
|
||||
out.User = (*placeholder.User)(unsafe.Pointer(in.User))
|
||||
out.Message = (*string)(unsafe.Pointer(in.Message))
|
||||
return nil
|
||||
}
|
||||
@ -213,7 +202,6 @@ func Convert_v1alpha1_LoginRequestStatus_To_placeholder_LoginRequestStatus(in *L
|
||||
|
||||
func autoConvert_placeholder_LoginRequestStatus_To_v1alpha1_LoginRequestStatus(in *placeholder.LoginRequestStatus, out *LoginRequestStatus, s conversion.Scope) error {
|
||||
out.Credential = (*LoginRequestCredential)(unsafe.Pointer(in.Credential))
|
||||
out.User = (*User)(unsafe.Pointer(in.User))
|
||||
out.Message = (*string)(unsafe.Pointer(in.Message))
|
||||
return nil
|
||||
}
|
||||
@ -242,25 +230,3 @@ func autoConvert_placeholder_LoginRequestTokenCredential_To_v1alpha1_LoginReques
|
||||
func Convert_placeholder_LoginRequestTokenCredential_To_v1alpha1_LoginRequestTokenCredential(in *placeholder.LoginRequestTokenCredential, out *LoginRequestTokenCredential, s conversion.Scope) error {
|
||||
return autoConvert_placeholder_LoginRequestTokenCredential_To_v1alpha1_LoginRequestTokenCredential(in, out, s)
|
||||
}
|
||||
|
||||
func autoConvert_v1alpha1_User_To_placeholder_User(in *User, out *placeholder.User, s conversion.Scope) error {
|
||||
out.Name = in.Name
|
||||
out.Groups = *(*[]string)(unsafe.Pointer(&in.Groups))
|
||||
return nil
|
||||
}
|
||||
|
||||
// Convert_v1alpha1_User_To_placeholder_User is an autogenerated conversion function.
|
||||
func Convert_v1alpha1_User_To_placeholder_User(in *User, out *placeholder.User, s conversion.Scope) error {
|
||||
return autoConvert_v1alpha1_User_To_placeholder_User(in, out, s)
|
||||
}
|
||||
|
||||
func autoConvert_placeholder_User_To_v1alpha1_User(in *placeholder.User, out *User, s conversion.Scope) error {
|
||||
out.Name = in.Name
|
||||
out.Groups = *(*[]string)(unsafe.Pointer(&in.Groups))
|
||||
return nil
|
||||
}
|
||||
|
||||
// Convert_placeholder_User_To_v1alpha1_User is an autogenerated conversion function.
|
||||
func Convert_placeholder_User_To_v1alpha1_User(in *placeholder.User, out *User, s conversion.Scope) error {
|
||||
return autoConvert_placeholder_User_To_v1alpha1_User(in, out, s)
|
||||
}
|
||||
|
@ -120,11 +120,6 @@ func (in *LoginRequestStatus) DeepCopyInto(out *LoginRequestStatus) {
|
||||
*out = new(LoginRequestCredential)
|
||||
(*in).DeepCopyInto(*out)
|
||||
}
|
||||
if in.User != nil {
|
||||
in, out := &in.User, &out.User
|
||||
*out = new(User)
|
||||
(*in).DeepCopyInto(*out)
|
||||
}
|
||||
if in.Message != nil {
|
||||
in, out := &in.Message, &out.Message
|
||||
*out = new(string)
|
||||
@ -158,24 +153,3 @@ func (in *LoginRequestTokenCredential) DeepCopy() *LoginRequestTokenCredential {
|
||||
in.DeepCopyInto(out)
|
||||
return out
|
||||
}
|
||||
|
||||
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
|
||||
func (in *User) DeepCopyInto(out *User) {
|
||||
*out = *in
|
||||
if in.Groups != nil {
|
||||
in, out := &in.Groups, &out.Groups
|
||||
*out = make([]string, len(*in))
|
||||
copy(*out, *in)
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new User.
|
||||
func (in *User) DeepCopy() *User {
|
||||
if in == nil {
|
||||
return nil
|
||||
}
|
||||
out := new(User)
|
||||
in.DeepCopyInto(out)
|
||||
return out
|
||||
}
|
||||
|
@ -120,11 +120,6 @@ func (in *LoginRequestStatus) DeepCopyInto(out *LoginRequestStatus) {
|
||||
*out = new(LoginRequestCredential)
|
||||
(*in).DeepCopyInto(*out)
|
||||
}
|
||||
if in.User != nil {
|
||||
in, out := &in.User, &out.User
|
||||
*out = new(User)
|
||||
(*in).DeepCopyInto(*out)
|
||||
}
|
||||
if in.Message != nil {
|
||||
in, out := &in.Message, &out.Message
|
||||
*out = new(string)
|
||||
@ -158,24 +153,3 @@ func (in *LoginRequestTokenCredential) DeepCopy() *LoginRequestTokenCredential {
|
||||
in.DeepCopyInto(out)
|
||||
return out
|
||||
}
|
||||
|
||||
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
|
||||
func (in *User) DeepCopyInto(out *User) {
|
||||
*out = *in
|
||||
if in.Groups != nil {
|
||||
in, out := &in.Groups, &out.Groups
|
||||
*out = make([]string, len(*in))
|
||||
copy(*out, *in)
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new User.
|
||||
func (in *User) DeepCopy() *User {
|
||||
if in == nil {
|
||||
return nil
|
||||
}
|
||||
out := new(User)
|
||||
in.DeepCopyInto(out)
|
||||
return out
|
||||
}
|
||||
|
@ -28,7 +28,6 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA
|
||||
"github.com/suzerain-io/placeholder-name/kubernetes/1.19/api/apis/placeholder/v1alpha1.LoginRequestSpec": schema_api_apis_placeholder_v1alpha1_LoginRequestSpec(ref),
|
||||
"github.com/suzerain-io/placeholder-name/kubernetes/1.19/api/apis/placeholder/v1alpha1.LoginRequestStatus": schema_api_apis_placeholder_v1alpha1_LoginRequestStatus(ref),
|
||||
"github.com/suzerain-io/placeholder-name/kubernetes/1.19/api/apis/placeholder/v1alpha1.LoginRequestTokenCredential": schema_api_apis_placeholder_v1alpha1_LoginRequestTokenCredential(ref),
|
||||
"github.com/suzerain-io/placeholder-name/kubernetes/1.19/api/apis/placeholder/v1alpha1.User": schema_api_apis_placeholder_v1alpha1_User(ref),
|
||||
"k8s.io/apimachinery/pkg/apis/meta/v1.APIGroup": schema_pkg_apis_meta_v1_APIGroup(ref),
|
||||
"k8s.io/apimachinery/pkg/apis/meta/v1.APIGroupList": schema_pkg_apis_meta_v1_APIGroupList(ref),
|
||||
"k8s.io/apimachinery/pkg/apis/meta/v1.APIResource": schema_pkg_apis_meta_v1_APIResource(ref),
|
||||
@ -365,12 +364,6 @@ func schema_api_apis_placeholder_v1alpha1_LoginRequestStatus(ref common.Referenc
|
||||
Ref: ref("github.com/suzerain-io/placeholder-name/kubernetes/1.19/api/apis/placeholder/v1alpha1.LoginRequestCredential"),
|
||||
},
|
||||
},
|
||||
"user": {
|
||||
SchemaProps: spec.SchemaProps{
|
||||
Description: "A User will be populated from the Identity Provider.",
|
||||
Ref: ref("github.com/suzerain-io/placeholder-name/kubernetes/1.19/api/apis/placeholder/v1alpha1.User"),
|
||||
},
|
||||
},
|
||||
"message": {
|
||||
SchemaProps: spec.SchemaProps{
|
||||
Description: "An error message will be returned for an unsuccessful login request.",
|
||||
@ -382,7 +375,7 @@ func schema_api_apis_placeholder_v1alpha1_LoginRequestStatus(ref common.Referenc
|
||||
},
|
||||
},
|
||||
Dependencies: []string{
|
||||
"github.com/suzerain-io/placeholder-name/kubernetes/1.19/api/apis/placeholder/v1alpha1.LoginRequestCredential", "github.com/suzerain-io/placeholder-name/kubernetes/1.19/api/apis/placeholder/v1alpha1.User"},
|
||||
"github.com/suzerain-io/placeholder-name/kubernetes/1.19/api/apis/placeholder/v1alpha1.LoginRequestCredential"},
|
||||
}
|
||||
}
|
||||
|
||||
@ -405,40 +398,6 @@ func schema_api_apis_placeholder_v1alpha1_LoginRequestTokenCredential(ref common
|
||||
}
|
||||
}
|
||||
|
||||
func schema_api_apis_placeholder_v1alpha1_User(ref common.ReferenceCallback) common.OpenAPIDefinition {
|
||||
return common.OpenAPIDefinition{
|
||||
Schema: spec.Schema{
|
||||
SchemaProps: spec.SchemaProps{
|
||||
Type: []string{"object"},
|
||||
Properties: map[string]spec.Schema{
|
||||
"name": {
|
||||
SchemaProps: spec.SchemaProps{
|
||||
Description: "Identity Provider name for authenticated user.",
|
||||
Type: []string{"string"},
|
||||
Format: "",
|
||||
},
|
||||
},
|
||||
"groups": {
|
||||
SchemaProps: spec.SchemaProps{
|
||||
Description: "Identity Provider groups for authenticated user.",
|
||||
Type: []string{"array"},
|
||||
Items: &spec.SchemaOrArray{
|
||||
Schema: &spec.Schema{
|
||||
SchemaProps: spec.SchemaProps{
|
||||
Type: []string{"string"},
|
||||
Format: "",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
Required: []string{"groups"},
|
||||
},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func schema_pkg_apis_meta_v1_APIGroup(ref common.ReferenceCallback) common.OpenAPIDefinition {
|
||||
return common.OpenAPIDefinition{
|
||||
Schema: spec.Schema{
|
||||
|
@ -7,6 +7,8 @@ package integration
|
||||
|
||||
import (
|
||||
"context"
|
||||
"crypto/x509"
|
||||
"encoding/pem"
|
||||
"net/http"
|
||||
"testing"
|
||||
"time"
|
||||
@ -77,9 +79,6 @@ func TestSuccessfulLoginRequest(t *testing.T) {
|
||||
require.NotEmpty(t, response.Status.Credential.ClientKeyData)
|
||||
require.NotNil(t, response.Status.Credential.ExpirationTimestamp)
|
||||
require.InDelta(t, time.Until(response.Status.Credential.ExpirationTimestamp.Time), 1*time.Hour, float64(3*time.Minute))
|
||||
require.NotNil(t, response.Status.User)
|
||||
require.NotEmpty(t, response.Status.User.Name)
|
||||
require.Contains(t, response.Status.User.Groups, "tmc:member")
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
|
||||
defer cancel()
|
||||
@ -103,7 +102,7 @@ func TestSuccessfulLoginRequest(t *testing.T) {
|
||||
Subjects: []rbacv1.Subject{{
|
||||
Kind: rbacv1.UserKind,
|
||||
APIGroup: rbacv1.GroupName,
|
||||
Name: response.Status.User.Name,
|
||||
Name: getCommonName(t, response.Status.Credential.ClientCertificateData),
|
||||
}},
|
||||
RoleRef: rbacv1.RoleRef{
|
||||
Kind: "ClusterRole",
|
||||
@ -154,7 +153,6 @@ func TestFailedLoginRequestWhenTheRequestIsValidButTheTokenDoesNotAuthenticateTh
|
||||
|
||||
require.Empty(t, response.Spec)
|
||||
require.Nil(t, response.Status.Credential)
|
||||
require.Nil(t, response.Status.User)
|
||||
require.Equal(t, stringPtr("authentication failed"), response.Status.Message)
|
||||
}
|
||||
|
||||
@ -182,3 +180,13 @@ func TestLoginRequest_ShouldFailWhenRequestDoesNotIncludeToken(t *testing.T) {
|
||||
func stringPtr(s string) *string {
|
||||
return &s
|
||||
}
|
||||
|
||||
func getCommonName(t *testing.T, certPEM string) string {
|
||||
t.Helper()
|
||||
|
||||
pemBlock, _ := pem.Decode([]byte(certPEM))
|
||||
cert, err := x509.ParseCertificate(pemBlock.Bytes)
|
||||
require.NoError(t, err)
|
||||
|
||||
return cert.Subject.CommonName
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user