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

chore: bump charm revisions #273

Merged
merged 1 commit into from
Sep 11, 2024
Merged

chore: bump charm revisions #273

merged 1 commit into from
Sep 11, 2024

Conversation

natalian98
Copy link
Contributor

@natalian98 natalian98 commented Sep 4, 2024

This PR:

  • bumps charm revisions to prepare for 0.3 bundle release

fixes #268

@natalian98 natalian98 marked this pull request as ready for review September 4, 2024 14:47
@natalian98 natalian98 requested a review from a team as a code owner September 4, 2024 14:47
nsklikas
nsklikas previously approved these changes Sep 4, 2024
Copy link
Contributor

@nsklikas nsklikas left a comment

Choose a reason for hiding this comment

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

LGTM but I have a couple of questionts:

  • Shouldn't this include tests for the Kratos IdP?
  • Also should we try updating traefik?

@natalian98
Copy link
Contributor Author

LGTM but I have a couple of questionts:

  • Shouldn't this include tests for the Kratos IdP?

I'll work on this as part of https://warthogs.atlassian.net/browse/IAM-996

  • Also should we try updating traefik?

Updated, thanks

wood-push-melon
wood-push-melon previously approved these changes Sep 4, 2024
Copy link
Contributor

@nsklikas nsklikas left a comment

Choose a reason for hiding this comment

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

Not sure if my comment makes sense, but if we want other people to use this, we need to come up with a solution that has a stable API.

If you disagree, feel free to disregard it and maybe we can come up with a solution for this in another PR

oauth_tools/oauth_helpers.py Outdated Show resolved Hide resolved
@nsklikas
Copy link
Contributor

nsklikas commented Sep 6, 2024

If I deploy the bundle via:

juju deploy ./bundle-edge.yaml --trust

And the try to login (via the hydra cli client), I will get redirected to the login page:
Screenshot from 2024-09-06 14-01-03

And if then click Sign in with Security key, I will see:
Screenshot from 2024-09-06 14-01-40

But if I try to input some random email, I will get an invalid password error:
image

Is this expected, is it a bug we should open in the login UI or is something wrong with the kratos config?

AFAICT the error comes from this line

@natalian98
Copy link
Contributor Author

Is this expected, is it a bug we should open in the login UI or is something wrong with the kratos config?

AFAICT the error comes from this line

It is not expected, this can be reproduced if webauthn is disabled, otherwise it displays the correct message. Perhaps we should not show the "Sign in with security key" button at all in that scenario.
I opened an issue: canonical/identity-platform-login-ui#277

@nsklikas
Copy link
Contributor

nsklikas commented Sep 6, 2024

If I deploy the bundle, create an admin account via juju CLI, setup the authenticator and try login via Hydra:
image

I will end up in the totp page:
image

Now if I go back (without entering the otp), I will be asked only for my password (username is cached):
image

If I now try to reset my password, I will be asked to input my email:
image

But then I will be redirected to an error page:
image

shipperizer
shipperizer previously approved these changes Sep 11, 2024
@shipperizer shipperizer dismissed their stale review September 11, 2024 11:10

thought about it, we could remove the traefik update and have the oauth tools changes in a separate pr

@natalian98
Copy link
Contributor Author

As discussed with @nsklikas, I reverted the traefik revision bump to address oauth_tools updates in a separate PR.

@natalian98 natalian98 merged commit 4d5733a into main Sep 11, 2024
4 checks passed
@natalian98 natalian98 deleted the release-03 branch September 11, 2024 14:15
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.

Update kratos relation name
4 participants