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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues on OIDC callback function #631

Closed
derBroBro opened this issue Sep 12, 2020 · 2 comments 路 Fixed by #638
Closed

Issues on OIDC callback function #631

derBroBro opened this issue Sep 12, 2020 · 2 comments 路 Fixed by #638
Assignees
Labels
jwt/oidc JWT/OIDC auth method

Comments

@derBroBro
Copy link
Contributor

Hello all,

first of all thank you for this great library! Usualy it is just awesome to work with it! 馃コ

As we use OIDC for login, I was super happy to see this was implemented a while ago.
During the try to implement it as decribed in the docs, I faced several issues and I am now at a point, where I think there is a tiny issue in the oidc_callback function. 馃槙
On my testing is looks like the paramters are not sended in the GET request as they should .

The snipped where the issue happens should be this one:

params = {
            'state': state,
            'nonce': nonce,
            'code': code,
        }
        api_path = utils.format_url(
            '/v1/auth/{path}/oidc/callback',
            path=self.resolve_path(path),
        )
        return self._adapter.get(
            url=api_path,
            json=params,
        )

Source: https://github.com/hvac/hvac/blob/master/hvac/api/auth_methods/jwt.py#L364

I can fix my issue if I modify the code to:

        api_path = utils.format_url(
            '/v1/auth/{path}/oidc/callback?state={state}&nonce={nonce}&code={code}',
            path=self.resolve_path(path),
            state=state,
            nonce=nonce,
            code=code
        )

I think this is not the way you are looking for get requests with paramters, but (even with a little search) I found no parts in the code where it is done diffrently.
How would you preffer - Should I open a PR with this coding or is it easier if you change it directly?

Greetings from Stuttgart, Germany

Malte

@jeffwecan jeffwecan added the jwt/oidc JWT/OIDC auth method label Sep 21, 2020
@jeffwecan jeffwecan self-assigned this Sep 21, 2020
@jeffwecan
Copy link
Member

A PR would be welcome here if you'd like to submit one @derBroBro. Honestly, I was a bit fuzzy on exactly how the OIDC API routes would be leveraged within a Python application as I have not personally had such a need in the past. That clearly translated into some gaps in the implementation 馃槃.

@derBroBro
Copy link
Contributor Author

Thanks for the feedback and rhe awesome library!
Sure, will prepare a PR these days.馃槄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jwt/oidc JWT/OIDC auth method
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants