Cannot override headless inputs

[mecampbellsoup]

It would be nice, I think, if the headless API endpoints honored any ACCOUNT_FORMS overrides set in account configuration.

However, it seems that e.g. the parent classes of RequestPasswordResetInput are hard-coded to the base form: https://github.com/pennersr/django-allauth/blob/9440c0a9cef050be11bbad533ea95b11a5f45fee/allauth/headless/account/inputs.py#L107-L108

Could we do something similar to the BaseSignupForm pattern in order to honor form overrides? https://github.com/pennersr/django-allauth/blob/9440c0a9cef050be11bbad533ea95b11a5f45fee/allauth/headless/account/inputs.py#L22

Related to https://github.com/pennersr/django-allauth/issues/4011, I was hoping to put some custom Form.save() logic in here.

[pennersr]

I would prefer to avoid this:

• The headed and headless behavior would start to behave differently.
• It makes the inputs part of the public API, thus affecting e.g. semver, whereas the implementation should be mostly considered to be internal.
• "It would be nice, I think" -- given the above two points, it would really require a good use case to start doing this.

[mecampbellsoup]

I'm not following your bullet points... I'm simply looking for a way to override some part of the headless API endpoint/view.

I would say my proposal actually unifies the headed and headless APIs since any custom form behavior would, under my proposal, behave the same way. Today, if you specify a customization in ACCOUNT_FORMS['reset_password'], that customization only applies to the headed API endpoint. Is that really desirable?

Our desired UX is as follows:

• When a user without a usable password requests a password reset, the operation does not result in a password reset email (w/ key) being sent to them. We could focus on the reset password view instead (after they get the email, open the UI, and type in their new password), but we're subjecting them to extra steps that are guaranteed to be futile, so that doesn't seem ideal.

As of the current code base, the only thing I can think of doing (assuming we continue to use the headless API) is to override DefaultAccountAdapter.get_reset_password_from_key_url to, say, throw an exception, but that seems like a hack.

I have whipped up two POCs for your consideration:

1. Honoring ACCOUNT_FORMS['reset_password'] in its headless counterpart view; and
2. implementing HEADLESS_INPUTS overrides that are analogous to ACCOUNT_FORMS.

[pennersr]

Today, if you specify a customization in ACCOUNT_FORMS['reset_password'], that customization only applies to the headed API endpoint. Is that really desirable?

True. That is due to ResetPasswordForm being hardcoded over here:

class RequestPasswordResetInput(ResetPasswordForm, inputs.Input):
    pass

So my preference would be for that RequestPasswordResetInput to respect ACCOUNT_FORMS['reset_password']. This way, your modifications always apply to both headed and headless versions.

[pennersr]

So, that is actually what you have done over here:

https://github.com/pennersr/django-allauth/pull/4027/files

That looks like a good approach. Would be good if this was consistently applied to all the forms.

[mecampbellsoup]

So, that is actually what you have done over here:

#4027 (files)

That looks like a good approach. Would be good if this was consistently applied to all the forms.

OK cool, and you're sure you prefer that to https://github.com/pennersr/django-allauth/pull/4026? I kind of prefer #4026 myself... I'm signing off for the day but have a think on it and LMK and I'll whip it up!

I think if we go with the pattern illustrated by #4027 we may want to come up w/ a new term that means "both forms and inputs"... forminputs? 😄

[pennersr]

Thoughts:

• Is there at all good reason for headless inputs to behave differently from headed forms? I would expect not so, meaning, there should be no need to have different headless input logic.
• Altering forms with the intent of altering logic is quite indirect, and perhaps it would be better to try and move things to the adapter so that we can avoid this issue altogether. Where are you intervening? Note that if you alter clean_email() that impacts enumeration prevention -- so I am assuming you are altering the save() instead? What if that save() logic was inside the adapter?

[mecampbellsoup]

• Is there at all good reason for headless inputs to behave differently from headed forms? I would expect not so, meaning, there should be no need to have different headless input logic.

No, I don't think so. Taking it one step further, what are the reasons for the forms (headed) vs. inputs (headless) discrepancy?

• Altering forms with the intent of altering logic is quite indirect, and perhaps it would be better to try and move things to the adapter so that we can avoid this issue altogether. Where are you intervening? Note that if you alter clean_email() that impacts enumeration prevention -- so I am assuming you are altering the save() instead?

Yes, exactly. In #4026, I'm intervening in MyCustomRequestPasswordResetInput.save() and in #4027, I'm intervening in MyCustomResetPasswordForm.save().

I tried altering DefaultAccountAdapter.clean_email but was unable to find a way to identify when it was being called from the ResetPasswordForm only as I don't want to e.g. prevent a new user signup when it's called in the signup form. Maybe we could change the API to add a kwarg such that you know about the caller but that feels potentially complicated and brittle at first blush; I would have to think more about that idea.

What if that save() logic was inside the adapter?

You mean, what if the ResetPasswordForm.save() logic was inside the adapter? Or "form save" logic generally?

---

FWIW one of the reasons I lean towards the approach taken in #4026 (allowing inputs customizations like the existing headed forms customizations) is that it side-steps the questions above and empowers users of this framework by letting them do what they think is best. But maybe there are tradeoffs to consider between the benefits of total control against the risks of breaking outside of the framework and footgunning oneself.

[pennersr]

what are the reasons for the forms (headed) vs. inputs (headless) discrepancy?

Not sure which discrepancy you are referring to? If you mean the fact that they cannot be configured like the headed counterparts, as mentioned, that is intentional as it would allow for logic to be different in headed vs headless. Additionally, I would like to keep the whole headless implementation under the wraps. As far as semver/API stability is concerned, the specification is the API, and the views and inputs are just implementation details.

You mean, what if the ResetPasswordForm.save() logic was inside the adapter? Or "form save" logic generally?

The former. So take ResetPasswordForm.save(), put its implementation on DefaultAccountAdapter in some method named request_password_reset().

[mecampbellsoup]

OK I think I'm following now. Let me try to summarize/ask anything still unclear to me:

1. Customizations to headless views could be effectuated via something like #4026 (settings.HEADLESS_INPUTS, analogous to headed settings.ACCOUNT_FORMS), but you don't like that idea because people might modify headless endpoints, thus introducing drift relative to their headed counterparts. The only part I don't really follow is that, to me, it seems like it's already possible to introduce drift and "allow for logic to be different in headed vs. headless", by way of settings.ACCOUNT_FORMS customizations. (This is exactly why I opened this issue: I was trying to use a form override to customize the functionality of headless request password reset and to my surprise my custom form wasn't being executed...)
2. Regarding #4027, you're open to this pattern but would prefer to instead enable request password reset functionality changes via something like DefaultAccountAdapter.request_password_rest() instead.

   ---

While we're discussing #4027: I was wondering whether all of the account forms that can be customized via ACCOUNT_FORMS were also ancestors to the corresponding headless inputs, and it seems like the answer is "partially yes". Here is a mapping of those forms to their inputs:

• add_email: allauth.account.forms.AddEmailForm is an ancestor to AddEmailInput ✅
• change_password: allauth.account.forms.ChangePasswordForm is NOT an ancestor to ChangePasswordInput ❌
• confirm_login_code: allauth.account.forms.ConfirmLoginCodeForm is an ancestor to ConfirmLoginCodeInput ✅
• login: allauth.account.forms.LoginForm is NOT an ancestor to LoginInput ❌
• request_login_code: allauth.account.forms.RequestLoginCodeForm is an ancestor to RequestLoginCodeInput ✅
• reset_password: allauth.account.forms.ResetPasswordForm is an ancestor to RequestPasswordResetInput ✅
• reset_password_from_key: allauth.account.forms.ResetPasswordKeyForm is NOT an ancestor to ResetPasswordKeyInput ❌
• set_password: ChangePasswordInput does NOT inherit allauth.account.forms.SetPasswordForm ❌
• signup: SignupInput inherits BaseSignupForm which honors ACCOUNT_SIGNUP_FORM_CLASS, but not ACCOUNT_FORMS['signup']... 🏁 🟡
• user_token: allauth.account.forms.UserTokenForm is also used within ResetPasswordKeyInput.clean_key ✅

[pennersr]

OK I think I'm following now. Let me try to summarize/ask anything still unclear to me:

With respect to 1, the drift is not the only point. Keeping the headless implementation private is also a big issue. When altering regular forms, users are just using the official Django forms API. The headless inputs/views/responses are not meant as a general reusable Django API framework. It's just an implementation detail needed to implement the allauth headless OpenAPI specification in a manner that is free of django-restframework/django-ninja dependencies. As mentioned, if possible, I would really prefer to keep it that way (internal implementation detail).

Our desired UX is as follows: When a user without a usable password requests a password reset, the operation does not result in a password reset email (w/ key) being sent to them

What does happen instead? Note that if you don't "pretend" to send a password reset email, that effectively breaks enumeration prevention.

Here is a mapping of those forms to their inputs:

There are indeed red crosses there. But I am not sure we should aim for getting rid of all of them. Take SetPasswordForm -- there is simply no counterpart there, the headless API does not distinguish between 2 types of inputs (change vs set), so this is simply not unifiable as is now. There is no 1:1 relationship between headed forms and headless inputs. Also, the question is, what problems do these really cause in practice? Obviously, for the flow you are hitting here there is an issue, but we can also try and address these issues one by one as they arise.

[mecampbellsoup]

Our desired UX is as follows: When a user without a usable password requests a password reset, the operation does not result in a password reset email (w/ key) being sent to them

What does happen instead? Note that if you don't "pretend" to send a password reset email, that effectively breaks enumeration prevention.

Right; we will still respond with something like "Success, check your email!" to prevent enumeration. But we will most likely send them an email explaining, "You attempted to reset your password, but your organization does not permit setting of passwords: the following login methods are available to you: SAML, Google, Github..."

Also, the question is, what problems do these really cause in practice? Obviously, for the flow you are hitting here there is an issue, but we can also try and address these issues one by one as they arise.

Yep, I agree. I just wanted to explore the prospect of a more unified set of APIs. It definitely seems premature for that though, both in terms of my own understanding of the headless API and what role the inputs play, and just the overall YAGNI/KISS mantras blaring in my mind...

Thank you for humoring my POCs/ideas 😄

[pennersr]

Closing now that #4034 is merged.