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

feat(PasswordAuthentication): Registration and authentication with local credentials #4

Merged
merged 21 commits into from
Oct 24, 2022

Conversation

jimsynz
Copy link
Collaborator

@jimsynz jimsynz commented Oct 5, 2022

This is missing a bunch of features that you probably want to use (eg confirmation, password resets), but it's a pretty good place to put a stake in the sand and say it works.

@zachdaniel
Copy link
Collaborator

This is 🔥 🔥 🔥

.formatter.exs Outdated Show resolved Hide resolved
@jimsynz jimsynz changed the title wip: continue work on an identity provider. feat(Identity): Basic identity registration and sign up is working. Oct 13, 2022
@jimsynz jimsynz marked this pull request as ready for review October 13, 2022 02:20
name: :authentication,
describe: "Configure authentication for this resource",
schema: [
subject_name: [
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is subject_name really generic to all authentication? i.e some tokens might not use "subjects". I guess some kind of identifier is though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I couldn't think of a more generic term for the concept. Happy to take suggestions :)

@@ -27,6 +27,8 @@ defmodule AshAuthentication.Plug do
|> Keyword.fetch!(:otp_app)
|> Macro.expand_literal(__ENV__)

AshAuthentication.Validations.validate_unique_subject_names(otp_app)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately, doing this at compile time is a harder task than it would seem. This won't necessarily get all resources because of the registry/api split. The split between registries and APIs was necessary in order to prevent massive compile time blockages/deadlocks, but it also makes validations based on enumerating resources like this, sadly undeterministic. Partial recompiles can result in this module not recompiling and therefore it can be out of date. Other things solve for this by requiring the registry be explicitly stated, and using Registry.Info.entries/1: https://github.com/ash-project/ash_graphql/blob/main/documentation/tutorials/getting-started-with-graphql.md#add-ashgraphql-to-your-schema

Also happy to see some experimentation here around using things like Api.Info.registry(api) |> Code.ensure_compiled!() or Api.Info.registry(api).spark_dsl_config() to see if that causes the proper compile time dependency (from this module to the registry of all configured api module). If that works then I'd prefer to take that approach elsewhere also.

@jimsynz jimsynz force-pushed the feat/identity branch 3 times, most recently from f8a14ce to 52fcf7d Compare October 21, 2022 00:35
jimsynz and others added 17 commits October 21, 2022 13:37
This is missing a bunch of features that you probably want to use (eg confirmation, password resets), but it's a pretty good place to put a stake in the sand and say it works.
Co-authored-by: Mike Buhot <[email protected]>
This should help when we're passed the session as just a map, for example in LiveView.
@jimsynz jimsynz force-pushed the feat/identity branch 2 times, most recently from a4e7f73 to 6ac1c6a Compare October 21, 2022 00:55
@jimsynz jimsynz changed the title feat(Identity): Basic identity registration and sign up is working. feat(PasswordAuthentication): Registration and authentication with local credentials Oct 24, 2022
@jimsynz jimsynz merged commit a939dde into main Oct 24, 2022
@jimsynz jimsynz deleted the feat/identity branch October 24, 2022 22:07
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.

None yet

3 participants