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

Potentially incorrect use of DRF's APISettings that makes it impossible to add a custom 2FA backend #135

Open
niespodd opened this issue Mar 24, 2022 · 4 comments · May be fixed by #201
Open
Assignees

Comments

@niespodd
Copy link

I came across this when I tried to add a custom 2FA backend.

The library uses APISettings from DRF as a base class for TrenchAPISettings:

class TrenchAPISettings(APISettings):
    _FIELD_USER_SETTINGS = "_user_settings"
    _FIELD_TRENCH_AUTH = "TRENCH_AUTH"

This wouldn't be a big deal if the TrenchAPISettings would not parse defaults during config load. Effectively, making it impossible to add any new custom backends.

There is a silent KeyError thrown by for k, v in self.defaults[self._FIELD_MFA_METHODS][method_name].items(): and the only way to get around it is to monkey-patch the constant and override trench_settings.

It is worth mentioning that aforementioned DRF's APISettings explicitly states within its implementation that:

    This is an internal class that is only compatible with settings namespaced
    under the REST_FRAMEWORK name. It is not intended to be used by 3rd-party
    apps, and test helpers like `override_settings` may not work as expected.
@niespodd niespodd changed the title Unclear and incorrect use of DRF's APISettings that makes it impossible to add a custom 2FA backend Potentially incorrect use of DRF's APISettings that makes it impossible to add a custom 2FA backend Mar 25, 2022
@allow-cookies allow-cookies self-assigned this Mar 25, 2022
@allow-cookies
Copy link
Collaborator

Hi, thank you for noticing this issue and reporting it. You've made a good point about using the DRF's Settings object. We will try to address this issue in the following release.

@jaimefma
Copy link

jaimefma commented Jun 3, 2022

That would be awesome!

@wmaciejewskimer wmaciejewskimer linked a pull request Dec 29, 2022 that will close this issue
@jamievleeshouwer
Copy link

Is this PR going to get merged? It has been sitting for more than a year.

Similarly, I am also unable to add a custom backend because of this issue.

@jamievleeshouwer
Copy link

A workaround until this is fixed is to do a monkey patch as @niespodd mentions.

This is the snippet I have used if anyone needs code examples...

import trench.settings
trench.settings.DEFAULTS['MFA_METHODS']['your_custom_method'] = {}
...Keep on doing cool code

It sets a key with a blank dictionary in the default settings (to avoid the KeyError)
Because the KeyError is now avoided, your custom method will now be populated with the data from your settengs.py as the doco describes.

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 a pull request may close this issue.

4 participants