August-W: Made ServiceProvider more configurable without needing to extend it #20
Conversation
…login_return_endpoint, and acs_redirect_endpoint, fixed issue with scheme in views to allow for https, and updated the example sp.py accordingly
…eturn_url and get_logout_return_url in sp.py
… use their chosen value as entity_id rather than the default url to an xml file
|
@timheap can we merge this? |
| def create_blueprint(self, login_endpoint='/login/', login_idp_endpoint='/login/idp/', | ||
| logout_endpoint='/logout/', acs_endpoint='/acs/', sls_endpoint='/sls/', | ||
| metadata_endpoint='/metadata.xml', scheme='http') -> Blueprint: |
There was a problem hiding this comment.
I’m far from sure about this, but shouldn’t the new entity_id not also be passed as an argument here, same as the scheme? (And then of course also something at line 345)
| def create_blueprint(self, login_endpoint='/login/', login_idp_endpoint='/login/idp/', | |
| logout_endpoint='/logout/', acs_endpoint='/acs/', sls_endpoint='/sls/', | |
| metadata_endpoint='/metadata.xml', scheme='http') -> Blueprint: | |
| def create_blueprint(self, login_endpoint='/login/', login_idp_endpoint='/login/idp/', | |
| logout_endpoint='/logout/', acs_endpoint='/acs/', sls_endpoint='/sls/', | |
| metadata_endpoint='/metadata.xml', scheme='http', entity_id=None) -> Blueprint: |
There was a problem hiding this comment.
I thought about that. The main reason I didn't pass entity_id there is that it seemed to be a little outside the scope of what this create_blueprint function should do. To me, it makes sense to set the scheme together with the various endpoints, as they are related. But you could make a case that even scheme doesn't belong here, as it is setting a value in sp rather than in idp_bp.
I'm open to including entity_id or removing scheme. Maybe creating a separate config_and_create_blueprint function which sets the entity_id and scheme and then calls create_blueprint. Or other ideas? Not sure what the cleanest solution is.
There was a problem hiding this comment.
Aah, yeah, got it. In that case I’m out of my depth, I wouldn’t know what the cleanest solution would be I’m afraid.
|
Is there an update on this? I'm thinking of using this library, but not being able to use https as the scheme is preventing me from using it |
|
Hey @lucasSzavara, I'm not sure if this is getting merged but for now you can just extend ServiceProvider and Login and make the same changes I did here, or you can use my fork |
|
Thanks @August-W! I'll use your fork, is there anything that I need to change on my code, other than including the scheme parameter? |
|
No problem, @lucasSzavara. You shouldn't have to change anything else. It's been a while since I've looked at this but let me know if you run into any issues. |
Added class variables in ServiceProvider for logout_endpoint, login_return_endpoint, entity_id, and acs_redirect_endpoint, and added parameters in the create_blueprint method.
With acs_redirect_endpoint, you can explicitly set the relay_state in AssertionConsumer, for cases in which the SAML Request does not contain a relay_state parameter.
Now, if you don't want to use a url to the saml xml file as your entity_id (default behaviour), you can set the entity_id in ServiceProvider.
Fixed an issue with the Login class in views.py. It now supports setting the scheme to "https" (this happens in ServiceProvider's create_blueprint method).
Updated the example sp.py accordingly.
Linked Issues:
#17
#18
#19