Skip to content

fixed Originating Identity example#559

Merged
mattmcneeney merged 2 commits intocloudfoundry:masterfrom
fmui:master
Jul 31, 2018
Merged

fixed Originating Identity example#559
mattmcneeney merged 2 commits intocloudfoundry:masterfrom
fmui:master

Conversation

@fmui
Copy link
Contributor

@fmui fmui commented Jul 23, 2018

The decoded value of the Originating Identity example also contains a "user_name" entry.

@cfdreddbot
Copy link

Hey fmui!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@duglin
Copy link
Contributor

duglin commented Jul 23, 2018

LGTM

Approved with PullApprove

Copy link
Contributor

@mattmcneeney mattmcneeney left a comment

Choose a reason for hiding this comment

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

None of the fields in the Originating Identity Header section of the profile describes a key named user_name. Did you mean username as described in the Kuberenetes section?

@duglin
Copy link
Contributor

duglin commented Jul 23, 2018

hmm the example is a CF one, so perhaps we need to drop that user_name field from the encoding? Does CF have a user_name field?

@mattmcneeney
Copy link
Contributor

@duglin Not according to the profiles (and not that I know of!)

@fmui
Copy link
Contributor Author

fmui commented Jul 23, 2018

@mattmcneeney If you decode the base64 string that is in the spec today, the JSON contains the field "user_name". We could either don't touch the base64 string and use "user_name" or we change the base64 string and JSON to something else.

@mattmcneeney
Copy link
Contributor

Interesting @fmui - I didn't know that! Can we use this PR to add that to the profile doc as well?

@Samze
Copy link
Contributor

Samze commented Jul 23, 2018

It looks like the docs are wrong to me @fmui. At least in CC we only send user_id. So decodes to:

{
  "user_id": "683ea748-3092-4ff4-b656-39cacc4d5360",
}

So it looks like the base64 hash is infact wrong.

@fmui
Copy link
Contributor Author

fmui commented Jul 23, 2018

I've changed the base64 strings.

@duglin
Copy link
Contributor

duglin commented Jul 24, 2018

LGTM

Approved with PullApprove

@zrob
Copy link

zrob commented Jul 31, 2018

lgtm

Approved with PullApprove

@mattmcneeney mattmcneeney merged commit 31bf79d into cloudfoundry:master Jul 31, 2018
@mattmcneeney mattmcneeney added this to the 2.15 milestone Jun 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants