test for authentication and oidc closes #120 #123

moritz · 2025-07-24T17:12:55+02:00

moritz commented

2025-07-24 17:12:55 +02:00

No description provided.

moritz self-assigned this 2025-07-24 17:12:55 +02:00

moritz added 2 commits 2025-07-24 17:12:56 +02:00

feat: fail if oidc provide does not provide a sub or id 9b2d0480d6

feat: test authentication

continuous-integration/drone/push Build is passing

Details

109427cf56

moritz added this to the Sprint 4 - 09.07 - 30.07 project 2025-07-24 17:13:09 +02:00

requested reviews from simon, carla, rafael

2025-07-24 17:13:17 +02:00

carla approved these changes 2025-07-28 10:10:32 +02:00

lib/accounts/accounts.ex Outdated

					
				@ -15,6 +15,8 @@ defmodule Mv.Accounts do

				      define :list_users, action: :read

				      define :update_user, action: :update_user

				      define :destroy_user, action: :destroy

				      define :create_register_with_rauthy, action: :register_with_rauthy

carla commented

2025-07-28 10:10:26 +02:00

Do I get it right, that the actions are just for the tests? Or do they have othe rpurposes?

rafael commented

2025-07-31 11:59:05 +02:00

Yes, this is just used in the tests as far as I can see. This line adds a convenience function for calling the register_with_rauthy action like this:

Mv.Accounts.create_register_with_rauthy(...)

Which is used in the OidcIntegrationTest module.

Yes, this is just used in the tests as far as I can see. This line adds a convenience function for calling the `register_with_rauthy` action like this: ```elixir Mv.Accounts.create_register_with_rauthy(...) ``` Which is used in the `OidcIntegrationTest` module.

👍 1

rafael approved these changes 2025-07-31 14:00:09 +02:00

rafael left a comment

This looks great and ready to merge :)

test/mv_web/controllers/auth_controller_test.exs Outdated

					
				@ -38,0 +52,4 @@

				      )

				      |> render_submit()

				    assert html =~ "Email or password was incorrect"

rafael commented

2025-07-31 12:13:03 +02:00

just a thought, how do these kinds of asserts interact with gettext? as long as we always use english as the locale for our tests, I assume we'll be fine.

In general, I try to avoid assertions that depend on arbitrary UI text like this, because it's annoying to update the tests when changing the UI. But I can't really think of an alternative here 😬

just a thought, how do these kinds of asserts interact with `gettext`? as long as we always use english as the locale for our tests, I assume we'll be fine. In general, I try to avoid assertions that depend on arbitrary UI text like this, because it's annoying to update the tests when changing the UI. But I can't really think of an alternative here 😬

carla commented

2025-07-31 14:15:21 +02:00

We should anyways check if the flash core component which is a div with role "alert" is accessible to screenreaders. But just an idea: it has the css class alert-error and we could test for that. But just a minor thing, overall I agree that the PR is ready to merge :)