Skip to content
This repository was archived by the owner on Dec 6, 2024. It is now read-only.

Conversation

hqtoan94
Copy link

I have already added the authentication, please double check and let me know if you found any issues

Copy link

@rafa-munoz rafa-munoz left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your contribution. Please, check my comments and see the Travis CI build, because one test is broken: JWTAuthenticationTests.test_post_valid_jwt_header. Maybe you want to use django.test.override_settings to modify JWT_AUTH_DISABLED?

Dockerfile.dev Outdated
WORKDIR /code
RUN pip install tox
ADD . /code
ADD . /code

Choose a reason for hiding this comment

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

This change is not necessary. Every file should end in a new line

image: dot_jwt
volumes:
- .:/code
- .:/code
Copy link

@rafa-munoz rafa-munoz Jul 10, 2018

Choose a reason for hiding this comment

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

This change is not necessary. Every file should end in a new line

if payload_enricher:
fn = import_string(payload_enricher)
extra_data = fn(request)
extra_data['username'] = request.POST.get('username')

Choose a reason for hiding this comment

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

Every request won't have a username. For example on refresh_token actions. It should not be inside this if payload_enricher and something like:

if request.POST.get('username'):
    extra_data['username'] = request.POST.get('username')

Copy link

@rafa-munoz rafa-munoz left a comment

Choose a reason for hiding this comment

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

Looks great, thanks a lot for your contribution!

@rafa-munoz rafa-munoz merged commit fe93732 into humanitec:master Jul 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants