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

Redirect uri on error raise #164

Closed
wants to merge 3 commits into from

Conversation

synasius
Copy link
Contributor

This is a proposal to solve Issue #162

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 83f18d7 on synasius:redirect-uri-error into 21d31ee on idan:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0%) when pulling 83f18d7 on synasius:redirect-uri-error into 21d31ee on idan:master.

@ib-lundgren
Copy link
Collaborator

Looks good but I think we should take it one step further. I think having the collected request parameters available in all OAuth2Error exceptions would be helpful, if not for anything else but debugging.

Maybe pass in a keyword arg request that we extract client_id, redirect_uri, scopes etc from?

class OAuth2Error(Exception):
  error = None

  def __init__(self, description=None, uri=None, state=None, status_code=400, request=None):
    ...
    if request:
      self.redirect_uri = request.redirect_uri
      etc.

@synasius
Copy link
Contributor Author

Oh, that would be nice.
Would you like to unpack all the params collected in request._params or only some?

@ib-lundgren
Copy link
Collaborator

Let's start with cherry picking a few and add more as we find it helpful. Start with client_id, redirect_uri, scopes, response_type and grant_type maybe? Feel free to add more as you see fit.

If we add code or a flavour of token we should probably make sure they are not accidentally logged anywhere, don't think that is the case though.

@synasius
Copy link
Contributor Author

👍 Fully agree with you! I can update this pull req or create a new one if you prefer

@ib-lundgren
Copy link
Collaborator

Whatever you prefer =) A new will make for a slightly cleaner commit log but that is a minor issue imo.

@synasius
Copy link
Contributor Author

I love clean logs so i'll go with a new pr. Back in 10 minutes

@synasius synasius closed this May 29, 2013
@synasius synasius deleted the redirect-uri-error branch May 29, 2013 12:33
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

3 participants