GoogleWebComponents / google-signin

Google Sign-in web component
https://elements.polymer-project.org/elements/google-signin
Other
279 stars 89 forks source link

Add openidPrompt property #130

Closed nodirt closed 8 years ago

nodirt commented 8 years ago

Add prompt property to and elements. Its docs:

  Space-delimited, case-sensitive list of strings that
  specifies whether the the user is prompted for reauthentication
  and/or consent. The defined values are:
    none: do not display authentication or consent pages.
      This value is mutually exclusive with the rest.
    login: always prompt the user for reauthentication.
    consent: always show consent screen.
    select_account: always show account selection page.
      This enables a user who has multiple accounts to select amongst
      the multiple accounts that they might have current sessions for.
  For more information, see "prompt" parameter description in
  https://openid.net/specs/openid-connect-basic-1_0.html#RequestParameters

Updated demo and used it to test.

Fixes #128

atotic commented 8 years ago

Nice patch, fits well with the existing code. My only issue is the parameter name: "Prompt". Without reading the docs, I'd assume propmt would modify prompt text. Instead, it is an obscure open_id option. Could we change it to openidPrompt? That name is scary enough it'd drive people who do not know what they are doing away.

nodirt commented 8 years ago

Renamed the property.

I am not experienced with pull requests. Do you want me to squash these commits into one or you are ok merging three commits?

atotic commented 8 years ago

I understand, github merging pushes into pull requests feels a bit too magical. Your request looks good. @ebidel this looks like a useful feature, and code lgtm. Can we merge it?

ebidel commented 8 years ago

@addyosmani 's the maintainer. He should have a look.

nodirt commented 8 years ago

Addressed comments

nodirt commented 8 years ago

Friendly ping

atotic commented 8 years ago

@nodirt do not worry. addy is busy, he'll get to it and most probably merge it in.