Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Client / Server Auth Refactor #126

Merged
merged 49 commits into from
Apr 28, 2015
Merged

Client / Server Auth Refactor #126

merged 49 commits into from
Apr 28, 2015

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Apr 19, 2015

...including password reset.

dbkr added 30 commits March 23, 2015 14:20
2) Change places where we mean unauthenticated to 401, not 403, in C/S v2: hack so it stays as 403 in v1 because web client relies on it.
…e expect, that's just fine. I added the user_id (as in database pkey) and it broke: no point testing what that comes out as: it's determined by the db.
… modifying (captcha setup is now purely on the HS).
…should fix it so that doesn't need the key in two different places)
 * Now only the auth part goes to fallback, not the whole operation
 * Auth fallback is a normal API endpoint, not a static page
 * Params like the recaptcha pubkey can just live in the config
Involves a little engineering on JsonResource so its servlets aren't always forced to return JSON. I should document this more, in fact I'll do that now.
if 'bind' in body and body['bind']:
logger.debug("Binding emails %s to %s" % (
threepid, auth_user.to_string()
))
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need to call "% ()" explicitly when logging:

logging.debug("Binding emails %s to %s", threepid, auth_user.to_string())

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, thanks for spotting.

defer.returnValue((False, ret, clientdict))

@defer.inlineCallbacks
def add_oob_auth(self, stagetype, authdict, clientip):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this could do with some doc string to explain what "add_oob_auth" does and what the return value means.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@NegativeMjark
Copy link
Contributor

LGTM

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