Fix/better user lib for authentication #58
Conversation
…uery params for login
tracim/lib/core/user.py
Outdated
|
|
||
| # Check methods | ||
|
|
||
| def user_with_email_exists(self, email: str): |
tracim/lib/core/user.py
Outdated
| if user.validate_password(password): | ||
| return user | ||
| else: | ||
| raise BadUserPassword() |
There was a problem hiding this comment.
why do you return an instance of exception here and a class 3 lines further?
| ) | ||
| new_user = api.get_user_with_context(user) | ||
| assert isinstance(new_user, UserInContext) | ||
| assert new_user.user == user |
There was a problem hiding this comment.
you should also check that the context is valid (eg: user avatar url, ...)
| assert isinstance(user, User) | ||
| assert user.email == 'admin@admin.admin' | ||
|
|
||
| def test_unit__authenticate_user___err__bad_password(self): |
There was a problem hiding this comment.
replace bad password by wrong password
| config=self.config, | ||
| ) | ||
| with pytest.raises(AuthenticationFailed): | ||
| api.authenticate_user('unknown_user', 'bad_password') |
There was a problem hiding this comment.
bad password means "un mot de passe de mauvaise qualité". wrong password means "un mot de passe erroné". you should use WrongPassword (even rename the exception BadPassword to WrongPassword)
| password = request.params['password'] | ||
| if not (email and password): | ||
| raise Exception | ||
| email = request.json_body['email'] |
There was a problem hiding this comment.
ask @buxx how to automatically fill an object from Schema in order not to have to fill yourself stuff from request.json_body.
| session=request.dbsession, | ||
| config=app_config, | ||
| ) | ||
| return uapi.authenticate_user(email, password) |
There was a problem hiding this comment.
after the object is filled using Schema, the code here will be something like:
return uapi.authenticate_user(login.email, login.password)
| self.login, | ||
| route_name='login', | ||
| ) | ||
| configurator.add_route('login', '/sessions/login', request_method='POST') # nopep8 |
There was a problem hiding this comment.
would be nice to defin routes directly as view method decorators. is it possible ?
There was a problem hiding this comment.
we previously decide to not rely on pyramid default decorator :
It's not in pyramid philosophy to have decorator who create both view and route at the same time like in some others web framework, pyramid explicitly distinct view and route. That's why such type of decorator is not a good idea.
Working with default pyramid decorator who are only view decorator doesn't simplify a lot code as we already need to set route in another place .
Class method view decorator doesn't work well with Hapic , see algoo/hapic#42
inkhey
changed the title
Depends on #56.
-> Remove swagger_ui view
-> Allow in_context params in many user_lib method (is it the best way to deal with this problem) to return UserInContext object.
-> Add autehnticate_user method in user_lib
-> Add tests+renaming for user_lib.