Skip to content
This repository has been archived by the owner on Oct 28, 2020. It is now read-only.

[bug 1386383] Login via admin panel raise error #1375

Closed
wants to merge 1 commit into from

Conversation

safwanrahman
Copy link
Contributor

@safwanrahman safwanrahman commented Aug 11, 2017

So there were some issues that was raising the SuspiciousOperation exception. It was fixed in mozilla-django-oidc==0.2.0. But things need to be fixed before upgrading the version 0.2.0. Therefore override the authenticate method in RemoAuthenticationBackend backend and port the changes.

The superuser should have access to all page and should not necessarily complete full profile. Fix that issues so local development superuser dont fall into problem.

Moreover, fixing a issue where display_name become blank if any user create a user form management command without any email.

@akatsoulas @johngian r?

@akatsoulas
Copy link
Contributor

akatsoulas commented Sep 11, 2017

Hey @safwanrahman, thank you for this PR and sorry for the long delay on reviewing this.

I tried to reproduce the issue by running everything from scratch in the master branch and I get no errors when I try to login in the admin interface. What I tried was:

  • docker-compose rm -fv
  • docker-compose build
  • docker-compose up
  • Connect to the remo_web_1 container and run the ./manage.py createsuperuser command.

Can you please add the steps to reproduce this?

@safwanrahman
Copy link
Contributor Author

@akatsoulas I have created a screencast of the problem, you can see it here.

So I have done following

  • docker-compose down -v (Remove all the containers with volume)
  • docker-compose up
  • connect the web container and run migration and create superuser
  • Try to login admin interface with provided credintials

@akatsoulas I think, your local dev environment has set up with Auth0 OIDC credentials. Therefore you can not reproduce it. Can you try to reproduce it with default configuration? maybe resetting the configuration?

@mastizada
Copy link

I updated mozilla-django-oidc to 0.3.1 and changed the name of the middleware as stated in its documentation. Login worked, had infinite redirect loop before adding user to Reps group.

mozilla_django_oidc.contrib.auth0.middleware.RefreshIDToken changed to mozilla_django_oidc.auth.OIDCAuthenticationBackend.

@MichaelKohler
Copy link
Member

I updated mozilla-django-oidc to 0.3.1 and changed the name of the middleware as stated in its documentation. Login worked, had infinite redirect loop before adding user to Reps group.

Without knowing these libraries too well, I think that would be a cleaner approach to this problem. Would you mind creating a PR for it and I'll test this out?

@akatsoulas, @safwanrahman what do you think?

@safwanrahman
Copy link
Contributor Author

safwanrahman commented Oct 2, 2017

@mastizada I have tried updating it but some tests seems to fail in mozilla-django-oidc==0.3.1

Therefore, I think updaing the library version maybe a solution, but its a big task to do!

@mastizada
Copy link

@safwanrahman After installing using docs I got few errors (manage.py test):

  • base/tests/__init__.py, line 37: AssertionError: 'main.jinja' != 'profiles_edit.jinja'
  • events/tests/test_views.py", line 119: 0 != 1
  • Other issues with email (FAILED (failures=6, skipped=5))

After updating to new version of mozilla-django-oidc I got same errors (failures=6, skipped=5).

May be its skipping oidc tests because I don't have credentials (OIDC_...) in env file?

@safwanrahman
Copy link
Contributor Author

@mastizada The tests does not depend on OIDC key. You can try running the test with the current version and you will see all the test are passing.

@Mte90
Copy link
Contributor

Mte90 commented Jan 3, 2018

any updates for this pr?

@Mte90
Copy link
Contributor

Mte90 commented Mar 2, 2018

It's funny that I wanted to ask for a ping and the last message on the pr is mine about a ping.

@Mte90
Copy link
Contributor

Mte90 commented Mar 13, 2018

Seems that now is not working anymore or it's me?

@MichaelKohler
Copy link
Member

What do you mean? Are you saying that the bug is not there anymore and it works as expected now?

@Mte90
Copy link
Contributor

Mte90 commented Mar 13, 2018

I am talking about the pr, I applied the code and the problem persist.

@MichaelKohler
Copy link
Member

As this should not be necessary anymore, I'm closing this PR. I will talk to tasos next week to see if we can eliminate the other pain point we have regarding local setup.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants