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

Fix messy setting (Operation Clean Config Project) #477

Merged
merged 18 commits into from
Feb 13, 2023
Merged

Fix messy setting (Operation Clean Config Project) #477

merged 18 commits into from
Feb 13, 2023

Conversation

kiraware
Copy link
Contributor

@kiraware kiraware commented Jan 31, 2023

Fix dj-rest-auth Messy Settings

This PR is created to make all dj-rest-auth settings stored in one dict variable called REST_AUTH. Also this PR related to Operation Clean Config Project. Please read the configuration docs here https://dj-rest-auth.readthedocs.io/en/latest/configuration.html

Summary

Before this PR

# old project settings.py
REST_AUTH_SERIALIZERS = {
    'LOGIN_SERIALIZER': 'dj_rest_auth.serializers.LoginSerializer',
    'TOKEN_SERIALIZER': 'dj_rest_auth.serializers.TokenSerializer',
    'JWT_SERIALIZER ': 'dj_rest_auth.serializers.JWTSerializer',
    'JWT_TOKEN_CLAIMS_SERIALIZER ': 'dj_rest_auth.serializers.LoginSerializer',
    'USER_DETAILS_SERIALIZER ': 'dj_rest_auth.serializers.TokenSerializer',
    'PASSWORD_RESET_SERIALIZER ': 'dj_rest_auth.serializers.PasswordResetSerializer',
    'PASSWORD_RESET_CONFIRM_SERIALIZER ': 'dj_rest_auth.serializers.PasswordResetConfirmSerializer',
    'PASSWORD_CHANGE_SERIALIZER ': 'dj_rest_auth.serializers.PasswordChangeSerializer',
}
REST_AUTH_REGISTER_SERIALIZERS = {
    'REGISTER_SERIALIZER ': 'dj_rest_auth.registration.serializers.RegisterSerializer',
}
REST_AUTH_REGISTER_PERMISSION_CLASSES = ('rest_framework.permissions.AllowAny',)
REST_AUTH_TOKEN_MODEL  = 'rest_framework.authtoken.models.Token'
REST_AUTH_TOKEN_CREATOR  = dj_rest_auth.utils.default_create_token
REST_AUTH_PW_RESET_USE_SITES_DOMAIN  = False
REST_SESSION_LOGIN = True
REST_USE_JWT = False
JWT_AUTH_COOKIE = None
JWT_AUTH_REFRESH_COOKIE = None
JWT_AUTH_REFRESH_COOKIE_PATH = '/'
JWT_AUTH_SECURE': False
JWT_AUTH_HTTPONLY = True
JWT_AUTH_SAMESITE = 'Lax'
JWT_AUTH_RETURN_EXPIRATION = False
OLD_PASSWORD_FIELD_ENABLED = False
LOGOUT_ON_PASSWORD_CHANGE = False
JWT_AUTH_COOKIE_USE_CSRF = False
JWT_AUTH_COOKIE_ENFORCE_CSRF_ON_UNAUTHENTICATED = False

After this PR

# new project settings.py
REST_AUTH = {
    'LOGIN_SERIALIZER': 'dj_rest_auth.serializers.LoginSerializer',
    'TOKEN_SERIALIZER': 'dj_rest_auth.serializers.TokenSerializer',
    'JWT_SERIALIZER': 'dj_rest_auth.serializers.JWTSerializer',
    'JWT_SERIALIZER_WITH_EXPIRATION': 'dj_rest_auth.serializers.JWTSerializerWithExpiration',
    'JWT_TOKEN_CLAIMS_SERIALIZER': 'rest_framework_simplejwt.serializers.TokenObtainPairSerializer',
    'USER_DETAILS_SERIALIZER': 'dj_rest_auth.serializers.UserDetailsSerializer',
    'PASSWORD_RESET_SERIALIZER': 'dj_rest_auth.serializers.PasswordResetSerializer',
    'PASSWORD_RESET_CONFIRM_SERIALIZER': 'dj_rest_auth.serializers.PasswordResetConfirmSerializer',
    'PASSWORD_CHANGE_SERIALIZER': 'dj_rest_auth.serializers.PasswordChangeSerializer',
    'REGISTER_SERIALIZER': 'dj_rest_auth.registration.serializers.RegisterSerializer',
    'REGISTER_PERMISSION_CLASSES': ('rest_framework.permissions.AllowAny',),
    'TOKEN_MODEL': 'rest_framework.authtoken.models.Token',
    'TOKEN_CREATOR': 'dj_rest_auth.utils.default_create_token',
    'PASSWORD_RESET_USE_SITES_DOMAIN': False,
    'SESSION_LOGIN': True,
    'USE_JWT': False,
    'JWT_AUTH_COOKIE': None,
    'JWT_AUTH_REFRESH_COOKIE': None,
    'JWT_AUTH_REFRESH_COOKIE_PATH': '/',
    'JWT_AUTH_SECURE': False,
    'JWT_AUTH_HTTPONLY': True,
    'JWT_AUTH_SAMESITE': 'Lax',
    'JWT_AUTH_RETURN_EXPIRATION': False,
    'OLD_PASSWORD_FIELD_ENABLED': False,
    'LOGOUT_ON_PASSWORD_CHANGE': False,
    'JWT_AUTH_COOKIE_USE_CSRF': False,
    'JWT_AUTH_COOKIE_ENFORCE_CSRF_ON_UNAUTHENTICATED': False,
}

What is changed?

  • dj-rest-auth Documentation
  • Your project settings.py
  • Some settings that prefixed with REST or REST_AUTH is shorted by removing those prefix
  • add @override_api_settings(JWT_AUTH_HTTPONLY=False) to tests.test_api.APIBasicTests.test_blacklisting since the default is True based on documentation, even though i found the default is False at this line, The value goes against the documentation.
  • add @override_api_settings(JWT_AUTH_HTTPONLY=False) to tests.test_api.APIBasicTests.test_rotate_token_refresh_view. And add @override_api_settings(SESSION_LOGIN=False) to tests.test_social.TestSocialConnectAuth.test_social_connect, tests.test_api.APIBasicTests.test_registration_allowed_with_custom_no_password_serializer, and fix tests.test_api.APIBasicTests.test_registration. I'm confused with this line, it looks like the default value of REST_SESSION_LOGIN or in new settings called SESSION_LOGIN is False. Once again the value goes against the documentation.

How to Fix?

Difficulty, NEED Help!!!

  • From my current commit in PR, I'm stuck with app_settings.py with these line

    if setting == 'REST_AUTH':
        api_settings = APISettings(value, DEFAULTS, IMPORT_STRINGS)

    The problem is setting value never changed to 'REST_AUTH', instead 'REST_FRAMEWORK'. How to fix this?

  • Make a better override_api_settings in tests.utils

  • Fix the documentation

@kiraware kiraware marked this pull request as draft January 31, 2023 07:50
@kiraware kiraware changed the title Fix messy setting Fix messy setting (Operation Clean Config Project) Feb 1, 2023
@kiraware kiraware marked this pull request as ready for review February 4, 2023 04:47
@kiraware
Copy link
Contributor Author

kiraware commented Feb 4, 2023

It's Ready for Review!!!

@iMerica and Anyone please review this PR :) Your suggestions matter!!!

@iMerica
Copy link
Owner

iMerica commented Feb 5, 2023

Thanks for submitting this pull request! Give me a day or two to review it please.

@kiraware
Copy link
Contributor Author

kiraware commented Feb 5, 2023

My pleasure sir!

@iMerica
Copy link
Owner

iMerica commented Feb 7, 2023

I think the reload_api_settings logic adds unnecessary scope to the goal of this pull request. Consider removing that.

@kiraware
Copy link
Contributor Author

kiraware commented Feb 8, 2023

I think the reload_api_settings logic adds unnecessary scope to the goal of this pull request. Consider removing that.

Done!

@iMerica iMerica merged commit cdbcf9c into iMerica:master Feb 13, 2023
lunika added a commit to openfun/marsha that referenced this pull request Feb 13, 2023
In dj-rest-auth version 3.0.0, settings have been refactor and are all
in the REST_AUTH setting. The REST or REST_AUTH prefix is removed for
thome of them.
Everything related to this change can be found in this PR
iMerica/dj-rest-auth#477
lunika added a commit to openfun/marsha that referenced this pull request Feb 14, 2023
In dj-rest-auth version 3.0.0, settings have been refactor and are all
in the REST_AUTH setting. The REST or REST_AUTH prefix is removed for
thome of them.
Everything related to this change can be found in this PR
iMerica/dj-rest-auth#477
lunika added a commit to openfun/marsha that referenced this pull request Feb 14, 2023
In dj-rest-auth version 3.0.0, settings have been refactor and are all
in the REST_AUTH setting. The REST or REST_AUTH prefix is removed for
thome of them.
Everything related to this change can be found in this PR
iMerica/dj-rest-auth#477
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

2 participants