Skip to content

Conversation

@ahardin-rh
Copy link
Contributor

@ahardin-rh ahardin-rh commented Apr 22, 2019

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 22, 2019
@ahardin-rh ahardin-rh self-assigned this Apr 22, 2019
@ahardin-rh
Copy link
Contributor Author

Removing login and challenge parameters per discussion with @spadgett

@spadgett @enj PTAL

cc @huffmanca FYI

@spadgett
Copy link
Member

lgtm, but would be good to have @enj review

name: idp-secret
claims: <7>
claims: <5>
preferredUsername:
- preferred_username
name:
- name
email:
- email
urls:
Copy link

Choose a reason for hiding this comment

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

urls was dropped openshift/api#265 and replaced with issuer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching that! This is now updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussed this with @huffmanca and he will provide further edits in a follow-up PR

@enj
Copy link

enj commented Apr 23, 2019

LGTM, feel free to update OpenID in a different PR

@ahardin-rh ahardin-rh force-pushed the login-challenge-removal branch from 6ae855f to c2fbf78 Compare April 23, 2019 14:04
@huffmanca huffmanca added the peer-review-done Signifies that the peer review team has reviewed this PR label Apr 23, 2019
@huffmanca huffmanca self-requested a review April 23, 2019 14:56
@huffmanca
Copy link
Contributor

This looks good to me! I'll handle OpenID in a separate PR.

@ahardin-rh
Copy link
Contributor Author

@stuartchuan Can you please provide QE review?

@ahardin-rh
Copy link
Contributor Author

@barleyer Can you please provide QE review?

@barleyer
Copy link

/lgtm

Need an another PR to update openid issuer field.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 25, 2019
@ahardin-rh
Copy link
Contributor Author

Thank you!

@ahardin-rh ahardin-rh merged commit 32878b8 into openshift:enterprise-4.1 Apr 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.1 lgtm Indicates that a PR is ready to be merged. peer-review-done Signifies that the peer review team has reviewed this PR size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants