Skip to content

Commit

Permalink
fix: provide more information in the retrieve idp information (#5927)
Browse files Browse the repository at this point in the history 
* fix: provide more information in the retrieve idp information

* change raw_information to proto struct

* change unmarshal

* improve description
  • Loading branch information
@livio-a
livio-a committed Jun 20, 2023
1 parent 83da9ca commit 1017568
Show file tree
Hide file tree
Showing 9 changed files with 169 additions and 52 deletions.
31 changes: 31 additions & 0 deletions internal/api/grpc/fields.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,19 @@ package grpc
import (
"testing"

"github.com/stretchr/testify/require"
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/types/known/structpb"
)

var CustomMappers = map[protoreflect.FullName]func(testing.TB, protoreflect.ProtoMessage) any{
"google.protobuf.Struct": func(t testing.TB, msg protoreflect.ProtoMessage) any {
e, ok := msg.(*structpb.Struct)
require.True(t, ok)
return e.AsMap()
},
}

// AllFieldsSet recusively checks if all values in a message
// have a non-zero value.
func AllFieldsSet(t testing.TB, msg protoreflect.Message, ignoreTypes ...protoreflect.FullName) {
Expand Down Expand Up @@ -36,3 +46,24 @@ func AllFieldsSet(t testing.TB, msg protoreflect.Message, ignoreTypes ...protore
}
}
}

func AllFieldsEqual(t testing.TB, expected, actual protoreflect.Message, customMappers map[protoreflect.FullName]func(testing.TB, protoreflect.ProtoMessage) any) {
md := expected.Descriptor()
name := md.FullName()
if mapper := customMappers[name]; mapper != nil {
require.Equal(t, mapper(t, expected.Interface()), mapper(t, actual.Interface()))
return
}

fields := md.Fields()

for i := 0; i < fields.Len(); i++ {
fd := fields.Get(i)

if fd.Kind() == protoreflect.MessageKind {
AllFieldsEqual(t, expected.Get(fd).Message(), actual.Get(fd).Message(), customMappers)
} else {
require.Equal(t, expected.Get(fd).Interface(), actual.Get(fd).Interface())
}
}
}
20 changes: 15 additions & 5 deletions internal/api/grpc/user/v2/user.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"io"

"golang.org/x/text/language"
"google.golang.org/protobuf/types/known/structpb"
"google.golang.org/protobuf/types/known/timestamppb"

"github.com/zitadel/zitadel/internal/api/authz"
Expand Down Expand Up @@ -64,8 +65,8 @@ func addUserRequestToAddHuman(req *user.AddHumanUserRequest) (*command.AddHuman,
for i, link := range req.GetIdpLinks() {
links[i] = &command.AddLink{
IDPID: link.GetIdpId(),
IDPExternalID: link.GetIdpExternalId(),
DisplayName: link.GetDisplayName(),
IDPExternalID: link.GetUserId(),
DisplayName: link.GetUserName(),
}
}
return &command.AddHuman{
Expand Down Expand Up @@ -124,8 +125,8 @@ func (s *Server) AddIDPLink(ctx context.Context, req *user.AddIDPLinkRequest) (_
orgID := authz.GetCtxData(ctx).OrgID
details, err := s.command.AddUserIDPLink(ctx, req.UserId, orgID, &domain.UserIDPLink{
IDPConfigID: req.GetIdpLink().GetIdpId(),
ExternalUserID: req.GetIdpLink().GetIdpExternalId(),
DisplayName: req.GetIdpLink().GetDisplayName(),
ExternalUserID: req.GetIdpLink().GetUserId(),
DisplayName: req.GetIdpLink().GetUserName(),
})
if err != nil {
return nil, err
Expand Down Expand Up @@ -176,6 +177,12 @@ func intentToIDPInformationPb(intent *command.IDPIntentWriteModel, alg crypto.En
return nil, err
}
}
rawInformation := new(structpb.Struct)
err = rawInformation.UnmarshalJSON(intent.IDPUser)
if err != nil {
return nil, err
}

return &user.RetrieveIdentityProviderInformationResponse{
Details: &object_pb.Details{
Sequence: intent.ProcessedSequence,
Expand All @@ -189,7 +196,10 @@ func intentToIDPInformationPb(intent *command.IDPIntentWriteModel, alg crypto.En
IdToken: idToken,
},
},
IdpInformation: intent.IDPUser,
IdpId: intent.IDPID,
UserId: intent.IDPUserID,
UserName: intent.IDPUserName,
RawInformation: rawInformation,
},
}, nil
}
Expand Down
62 changes: 38 additions & 24 deletions internal/api/grpc/user/v2/user_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@ import (
"github.com/stretchr/testify/require"
"github.com/zitadel/oidc/v2/pkg/oidc"
"golang.org/x/oauth2"
"google.golang.org/protobuf/types/known/structpb"
"google.golang.org/protobuf/types/known/timestamppb"

"github.com/zitadel/zitadel/internal/api/authz"
"github.com/zitadel/zitadel/internal/api/grpc"
"github.com/zitadel/zitadel/internal/command"
"github.com/zitadel/zitadel/internal/idp/providers/oauth"
openid "github.com/zitadel/zitadel/internal/idp/providers/oidc"
"github.com/zitadel/zitadel/internal/integration"
"github.com/zitadel/zitadel/internal/repository/idp"
object "github.com/zitadel/zitadel/pkg/grpc/object/v2alpha"
Expand Down Expand Up @@ -81,12 +83,15 @@ func createSuccessfulIntent(t *testing.T, idpID string) (string, string, time.Ti
intentID := createIntent(t, idpID)
writeModel, err := Tester.Commands.GetIntentWriteModel(ctx, intentID, Tester.Organisation.ID)
require.NoError(t, err)
idpUser := &oauth.UserMapper{
RawInfo: map[string]interface{}{
"id": "id",
idpUser := openid.NewUser(
&oidc.UserInfo{
Subject: "id",
UserInfoProfile: oidc.UserInfoProfile{
PreferredUsername: "username",
},
},
}
idpSession := &oauth.Session{
)
idpSession := &openid.Session{
Tokens: &oidc.Tokens[*oidc.IDTokenClaims]{
Token: &oauth2.Token{
AccessToken: "accessToken",
Expand Down Expand Up @@ -386,9 +391,9 @@ func TestServer_AddHumanUser(t *testing.T) {
},
IdpLinks: []*user.IDPLink{
{
IdpId: "idpID",
IdpExternalId: "externalID",
DisplayName: "displayName",
IdpId: "idpID",
UserId: "userID",
UserName: "username",
},
},
},
Expand Down Expand Up @@ -433,9 +438,9 @@ func TestServer_AddHumanUser(t *testing.T) {
},
IdpLinks: []*user.IDPLink{
{
IdpId: idpID,
IdpExternalId: "externalID",
DisplayName: "displayName",
IdpId: idpID,
UserId: "userID",
UserName: "username",
},
},
},
Expand Down Expand Up @@ -495,9 +500,9 @@ func TestServer_AddIDPLink(t *testing.T) {
&user.AddIDPLinkRequest{
UserId: "userID",
IdpLink: &user.IDPLink{
IdpId: idpID,
IdpExternalId: "externalID",
DisplayName: "displayName",
IdpId: idpID,
UserId: "userID",
UserName: "username",
},
},
},
Expand All @@ -511,9 +516,9 @@ func TestServer_AddIDPLink(t *testing.T) {
&user.AddIDPLinkRequest{
UserId: Tester.Users[integration.OrgOwner].ID,
IdpLink: &user.IDPLink{
IdpId: "idpID",
IdpExternalId: "externalID",
DisplayName: "displayName",
IdpId: "idpID",
UserId: "userID",
UserName: "username",
},
},
},
Expand All @@ -527,9 +532,9 @@ func TestServer_AddIDPLink(t *testing.T) {
&user.AddIDPLinkRequest{
UserId: Tester.Users[integration.OrgOwner].ID,
IdpLink: &user.IDPLink{
IdpId: idpID,
IdpExternalId: "externalID",
DisplayName: "displayName",
IdpId: idpID,
UserId: "userID",
UserName: "username",
},
},
},
Expand Down Expand Up @@ -678,7 +683,17 @@ func TestServer_RetrieveIdentityProviderInformation(t *testing.T) {
IdToken: gu.Ptr("idToken"),
},
},
IdpInformation: []byte(`{"RawInfo":{"id":"id"}}`),
IdpId: idpID,
UserId: "id",
UserName: "username",
RawInformation: func() *structpb.Struct {
s, err := structpb.NewStruct(map[string]interface{}{
"sub": "id",
"preferred_username": "username",
})
require.NoError(t, err)
return s
}(),
},
},
wantErr: false,
Expand All @@ -693,8 +708,7 @@ func TestServer_RetrieveIdentityProviderInformation(t *testing.T) {
require.NoError(t, err)
}

require.Equal(t, tt.want.GetDetails(), got.GetDetails())
require.Equal(t, tt.want.GetIdpInformation(), got.GetIdpInformation())
grpc.AllFieldsEqual(t, got.ProtoReflect(), tt.want.ProtoReflect(), grpc.CustomMappers)
})
}
}
35 changes: 27 additions & 8 deletions internal/api/grpc/user/v2/user_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"github.com/muhlemmer/gu"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/types/known/structpb"
"google.golang.org/protobuf/types/known/timestamppb"

"github.com/zitadel/zitadel/internal/api/grpc"
Expand All @@ -21,6 +23,8 @@ import (
user "github.com/zitadel/zitadel/pkg/grpc/user/v2alpha"
)

var ignoreTypes = []protoreflect.FullName{"google.protobuf.Duration", "google.protobuf.Struct"}

func Test_hashedPasswordToCommand(t *testing.T) {
type args struct {
hashed *user.HashedPassword
Expand Down Expand Up @@ -128,8 +132,10 @@ func Test_intentToIDPInformationPb(t *testing.T) {
InstanceID: "instanceID",
ChangeDate: time.Date(2019, 4, 1, 1, 1, 1, 1, time.Local),
},
IDPID: "idpID",
IDPUser: []byte(`{"id": "id"}`),
IDPID: "idpID",
IDPUser: []byte(`{"userID": "idpUserID", "username": "username"}`),
IDPUserID: "idpUserID",
IDPUserName: "username",
IDPAccessToken: &crypto.CryptoValue{
CryptoType: crypto.TypeEncryption,
Algorithm: "enc",
Expand Down Expand Up @@ -158,8 +164,10 @@ func Test_intentToIDPInformationPb(t *testing.T) {
InstanceID: "instanceID",
ChangeDate: time.Date(2019, 4, 1, 1, 1, 1, 1, time.Local),
},
IDPID: "idpID",
IDPUser: []byte(`{"id": "id"}`),
IDPID: "idpID",
IDPUser: []byte(`{"userID": "idpUserID", "username": "username"}`),
IDPUserID: "idpUserID",
IDPUserName: "username",
IDPAccessToken: &crypto.CryptoValue{
CryptoType: crypto.TypeEncryption,
Algorithm: "enc",
Expand All @@ -184,8 +192,19 @@ func Test_intentToIDPInformationPb(t *testing.T) {
Oauth: &user.IDPOAuthAccessInformation{
AccessToken: "accessToken",
IdToken: gu.Ptr("idToken"),
}},
IdpInformation: []byte(`{"id": "id"}`),
},
},
IdpId: "idpID",
UserId: "idpUserID",
UserName: "username",
RawInformation: func() *structpb.Struct {
s, err := structpb.NewStruct(map[string]interface{}{
"userID": "idpUserID",
"username": "username",
})
require.NoError(t, err)
return s
}(),
},
},
err: nil,
Expand All @@ -196,9 +215,9 @@ func Test_intentToIDPInformationPb(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
got, err := intentToIDPInformationPb(tt.args.intent, tt.args.alg)
require.ErrorIs(t, err, tt.res.err)
assert.Equal(t, tt.res.resp, got)
grpc.AllFieldsEqual(t, got.ProtoReflect(), tt.res.resp.ProtoReflect(), grpc.CustomMappers)
if tt.res.resp != nil {
grpc.AllFieldsSet(t, got.ProtoReflect())
grpc.AllFieldsSet(t, got.ProtoReflect(), ignoreTypes...)
}
})
}
Expand Down
2 changes: 2 additions & 0 deletions internal/command/idp_intent.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ func (c *Commands) SucceedIDPIntent(ctx context.Context, writeModel *IDPIntentWr
ctx,
&idpintent.NewAggregate(writeModel.AggregateID, writeModel.ResourceOwner).Aggregate,
idpInfo,
idpUser.GetID(),
idpUser.GetPreferredUsername(),
userID,
accessToken,
idToken,
Expand Down
4 changes: 4 additions & 0 deletions internal/command/idp_intent_model.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ type IDPIntentWriteModel struct {
FailureURL *url.URL
IDPID string
IDPUser []byte
IDPUserID string
IDPUserName string
IDPAccessToken *crypto.CryptoValue
IDPIDToken string
UserID string
Expand Down Expand Up @@ -72,6 +74,8 @@ func (wm *IDPIntentWriteModel) reduceStartedEvent(e *idpintent.StartedEvent) {
func (wm *IDPIntentWriteModel) reduceSucceededEvent(e *idpintent.SucceededEvent) {
wm.UserID = e.UserID
wm.IDPUser = e.IDPUser
wm.IDPUserID = e.IDPUserID
wm.IDPUserName = e.IDPUserName
wm.IDPAccessToken = e.IDPAccessToken
wm.IDPIDToken = e.IDPIDToken
wm.State = domain.IDPIntentStateSucceeded
Expand Down
18 changes: 11 additions & 7 deletions internal/command/idp_intent_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -439,15 +439,17 @@ func TestCommands_SucceedIDPIntent(t *testing.T) {
event, _ := idpintent.NewSucceededEvent(
context.Background(),
&idpintent.NewAggregate("id", "ro").Aggregate,
[]byte(`{"RawInfo":{"id":"id"}}`),
[]byte(`{"sub":"id","preferred_username":"username"}`),
"id",
"username",
"",
&crypto.CryptoValue{
CryptoType: crypto.TypeEncryption,
Algorithm: "enc",
KeyID: "id",
Crypted: []byte("accessToken"),
},
"",
"idToken",
)
return event
}(),
Expand All @@ -458,18 +460,20 @@ func TestCommands_SucceedIDPIntent(t *testing.T) {
args{
ctx: context.Background(),
writeModel: NewIDPIntentWriteModel("id", "ro"),
idpSession: &oauth.Session{
idpSession: &openid.Session{
Tokens: &oidc.Tokens[*oidc.IDTokenClaims]{
Token: &oauth2.Token{
AccessToken: "accessToken",
},
IDToken: "idToken",
},
},
idpUser: &oauth.UserMapper{
RawInfo: map[string]interface{}{
"id": "id",
idpUser: openid.NewUser(&oidc.UserInfo{
Subject: "id",
UserInfoProfile: oidc.UserInfoProfile{
PreferredUsername: "username",
},
},
}),
},
res{
token: "aWQ",
Expand Down

1 comment on commit 1017568

@vercel
Copy link

@vercel vercel bot commented on 1017568 Jun 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

docs – ./

zitadel-docs.vercel.app
docs-zitadel.vercel.app
docs-git-main-zitadel.vercel.app

Please sign in to comment.