Changing policy based on sms being configured seems a little off to me. Having the setting only conditionally show based on it being configured seems fine, however what happens in this scenario:

1. Turn on sms
2. Add number
3. Turn off sms

I’m guessing the user would be unable to remove their contact number (or even see it), unless turning off sms deletes their contact number. In this case I think the user would still want to know it’s saved somewhere and could remove it.

It might be better for now to include some explanation text on those pages saying it’s only used for MFA if enabled, and leave the display of these as-is

In D25452#12901, @speck wrote:

I’m guessing the user would be unable to remove their contact number (or even see it), unless turning off sms deletes their contact number. In this case I think the user would still want to know it’s saved somewhere and could remove it.

@speck: Currently (without my patch), users can only disable a contact number of them; users could not and cannot remove or delete a contact number which they entered into Phorge. Thus my patch does not make the situation worse: users still will not be able to remove the contact number themselves (SMS enabled or not) but at least users cannot enter more contact numbers while SMS is not enabled in their Phorge installation.

Thanks for clarifying this behavior, it sounds like contact numbers in general need fleshed out quite a bit.

What do you think about not modifying policy based on the sms MFA turned on? Would it be possible to have that endpoint return 404 or similar without dynamically modifying policy? Or is there another example in the codebase for similar dynamic policy definition that can be referenced?

I am sorry, thanks for your last comment, I now understand your point. Yes, I guess we'd better return new Aphront404Response() instead of some permission based message. Like we'd do trying to access the URL of an uninstalled application. Now just need to find out how to do that :)