Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

State of OAuth.Implicit.Success is Nothing #21

Closed
manuscrypt opened this issue May 27, 2020 · 6 comments
Closed

State of OAuth.Implicit.Success is Nothing #21

manuscrypt opened this issue May 27, 2020 · 6 comments

Comments

@manuscrypt
Copy link

When using version 7.0.0, everything works as expected, except the value of state is always Nothing.
The query-string contains a param named state.
All the other strings are parsed.

Debug.log of the success-object:

success: { expiresIn = Just 3600, refreshToken = Nothing, scope = [""], state = Nothing, token = Bearer "eyJ0eX..." }

What am I missing? I looked at the implementation and that looks fine to me.
Thanks for any pointers.

@KtorZ
Copy link
Contributor

KtorZ commented May 27, 2020

This is surprising because the state is basically free-form so there's no reason for the parser to fail if the value is present in the query.
May I ask what your raw query looks like 🤔 ?

@manuscrypt
Copy link
Author

manuscrypt commented May 28, 2020

Certainly, here's what I pass to OAuth.Implicit.parseToken:

{ fragment = Just "access_token=eyJ0e...PPjuBg&token_type=bearer&expires_in=3600&state=z31j7AMHBiAHySvY8PvtcA==", host = "localhost", path = "/dashboard", port_ = Just 4200, protocol = Http, query = Just "client-request-id=d6b27f0f-dba7-4a00-bd4b-0080000000d4" }

http://localhost:4200/dashboard?client-request-id=d6b27f0f-dba7-4a00-bd4b-0080000000d4#access_token=eyJ0e...PPjuBg&token_type=bearer&expires_in=3600&state=z31j7AMHBiAHySvY8PvtcA==

Here is my parseAuth function, adapted from your google-example to within my elm-spa app:

parseAuth : Commands msg -> Model -> ( Model, Cmd Msg, Cmd msg )
parseAuth cmds model =
    let
        randomState =
            model.session.randomState
    in
    case OAuth.Implicit.parseToken (Debug.log "query" model.session.origin) of
        OAuth.Implicit.Empty ->
            ( model
            , Cmd.none
            , Cmd.none
            )

        OAuth.Implicit.Success success ->
            case randomState of
                Nothing ->
                    ( model
                    , Cmd.none
                    , clearUrl cmds
                    )

                Just jumble ->
                    if Debug.log "state" success.state /= Debug.log "jumble" (Just jumble) then
                        ( model
                        , Cmd.none
                        , clearUrl cmds
                        )

                    else
                        ( model
                        , Cmd.batch [ Ports.log (Maybe.withDefault "nothing-state" success.state), Ports.storeToken (String.replace "Bearer " "" (OAuth.tokenToString success.token)) ]
                        , clearUrl cmds
                        )

        OAuth.Implicit.Error error ->
            ( { model | errors = [ error ] }
            , Cmd.none
            , clearUrl cmds
            )

@KtorZ
Copy link
Contributor

KtorZ commented May 28, 2020

Interesting. So it seems that the Url.Parser.Query which is used to parse the fragment internally doesn't quite like the padding (==) of the base64 state. Since the = sign has a very specific semantic for queries, this makes sense. This will be a good opportunity to add some unit tests to this 😬

I also realize that since 7.0.0, the internal parsers are no longer exposed, which doesn't allow you to write a custom parser. That's very inconvenient. This needs fix.

@manuscrypt
Copy link
Author

manuscrypt commented May 28, 2020

Thank you for looking into it (and restoring my sanity). If you're going to touch it anyway, maybe i could sneakily suggest a couple of changes here? In order to get the auth-url I really wanted, I had to:

    authorization
        |> OAuth.Implicit.makeAuthorizationUrl
        |> Url.toString
        |> String.replace "response_type=token" "response_type=token+id_token"
        |> (\s -> String.append s "&resource=95777017-55a5-4bbb-b662-b6b1fbf91a31")
        |> Navigation.load

so, i had to modify response_type and add resource. otherwise, i would not get the correct access_token (both modifications necessary). it would also be nice, to be able to get to the id_token returned, which is supplied as part of the response.

Neither this issue, nor my suggested changes are hindering me to use your package for which I am grateful. Thanks!

@KtorZ
Copy link
Contributor

KtorZ commented May 28, 2020

id_token isn't part of OAuth 2.0 though. I believe you're dealing with a system that implements OpenID Connect ( https://openid.net/specs/openid-connect-core-1_0.html#ImplicitAuthResponse ) which is another standard built on top of OAuth. I've long thought of also implementing an OpenID Connect client library in Elm but the standard is big and I haven't found much time / or interest to be honest 😊 ...

I've purposely left the parsers in this library somewhat open, so you should be able to work around pretty easily by using parseTokenWith, and re-using some parsers of the library, while completing the OpenID Connect specifics parts with others. Although, as I mentioned above, some low-level parsers aren't exposed anymore which I'll fix very soon in a new release.

@blawlor
Copy link

blawlor commented Jun 6, 2021

Hi - I came across this issue after a few hours of head-scratching. Like the OP I'm happy that sanity is restored. But can I ask why the issue was closed? I'm using 7.0.1 and this problem is still present. Any suggestions for a workaround?

Actually - I guess using state that has length which is a multiple of 3?

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

No branches or pull requests

3 participants