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

Makes it easy to update the login url #80

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

massilva
Copy link

@massilva massilva commented Jun 8, 2019

Add possibility to change the URL login by django's settings.

  • It makes possible redirect to endpoint, e.g., /endpoint

Add possibility to change the target parameter

… value violates unique constraint "auth_user_username_key"

Get_or_create`s implementation uses `kwargs` params to get the object on database,
the backend implementation makes it possible for username and shib_user_params['username'] to be different.
To avoid the error, before get or create user, It's set username to shib_user_params['username'] value.
@bcail
Copy link
Contributor

bcail commented Jun 10, 2019

thanks, @massilva.

@@ -40,6 +40,9 @@ def authenticate(self, request, remote_user, shib_meta):
# instead we use get_or_create when creating unknown users since it has
# built-in safeguards for multiple threads.
if self.create_unknown_user:
if 'username' in shib_user_params:
username = shib_user_params['username']

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure we need this. The value of 'username' comes from REMOTE_USER by default, but that can be configured differently. And the values in shib_user_params can be configured also. Seems like you could just configure things so that you know that 'username' has the value that you want?

Copy link
Author

Choose a reason for hiding this comment

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

@bcail thanks by feedback.

In my situation the 'username' coming from username = self.clean_username(remote_user) is https://idp/shibboleth!https://myhost/shibboleth!U6gPKa2gVte5tTPdbbRL2IpWvAA= and coming from shib_user_params is user1.

The get function, used by get_or_create function, uses kwargs param to get the object from database instead of defaults param.

Due that, It's raised IntegrityError: duplicate key error from database.

To avoid this error I setted username = shib_user_params['username'].

My application allow user log in from shibboleth and not.

My doubt is whether this behavior is because of the bad configuration of the shibboleth or is it an atypical situation?

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you overriding self.header in ShibbolethRemoteUserMiddleware? Otherwise I think self.header should be REMOTE_USER. Your value there for username seems like something might be misconfigured. What is the value of your SHIB_ATTRIBUTE_MAP?

Copy link
Author

Choose a reason for hiding this comment

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

I am not overriding self.header.

SHIBBOLETH_ATTRIBUTE_MAP = { 
      "Shib-Identity-Provider": (True, "idp"),
      "Shib-Session-ID": (True, "sessionid"),
      "Shib-Handler": (True, "shibhandler"),
      "Shib-inetOrgPerson-cn": (True, "first_name"),
      "Shib-inetOrgPerson-mail": (True, "email"),
      "Shib-inetOrgPerson-sn": (True, "last_name"),
      "Shib-eduPerson-eduPersonPrincipalName": (True, "username"),
  }

Copy link
Contributor

@bcail bcail Jun 12, 2019

Choose a reason for hiding this comment

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

It seems like the value of REMOTE_USER should be 'user1', the same as it is in shib_user_params. Maybe there's an issue with your Shibboleth config? Note that you can check the value of REMOTE_USER by logging request.META, even without using this package. Make sure the value of REMOTE_USER is what you expect.

As a work-around, you might be able to subclass ShibbolethRemoteUserMiddleware and set header = "Shib-eduPerson-eduPersonPrincipalName". See details and warning: https://docs.djangoproject.com/en/2.2/howto/auth-remote-user/#configuration.

Copy link
Author

Choose a reason for hiding this comment

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

I printed request,remote_user, username and shib_user_params['username']:

Why request is None? Maybe there's an issue with Shibboleth config?

image

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, there could be an issue with the Shib config. I would suggest testing your shib setup without this package at all - you can create a small test django project, and just use a basic view to look at the values of REMOTE_USER and the headers you have in shib_user_params. Make sure that all is correct, and then you can work on configuring this package.

LOGIN_URL_PARAMETER = getattr(settings, 'SHIBBOLETH_LOGIN_URL_PARAMETER', 'target')
#This should look like: /login
#This is the LOGIN_URL_PARAMETER value. The endpoint after login
LOGIN_REDIRECT_URL = getattr(settings, 'SHIBBOLETH_LOGIN_REDIRECT_URL', None)
Copy link
Contributor

Choose a reason for hiding this comment

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

Tests for these LOGIN settings would be nice.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, I will do.

Base automatically changed from master to main January 14, 2021 16:28
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.

2 participants