~AuthenticationService#getAccessToken(String cliendId, String clientSecret, String code, String redirectURI) - the authorization code should be cleared in the DB so that it is guaranteed it is single-use
I make it through set the ac_expiration to 0,if the user request the expired code again,invalid_grant would be reponsed~
~AuthenticationService#getAuthorizationCode - UUID() method generates unique but consecutive values thus enabling the attacker to guess the past and future codes. It is possible to improve randomization of the code while maintaining uniquiness constraint by applying a block cipher with a pre-configured secret and nonce. The UUID length is 128 bits thus any variation of AES would work
Could you please describe the detail of the applying a block cipher with a pre-configured secret and nonce? The result after encryption is unique too ? Should I configure the aes key in one properties file ?~
~AuthorizationOnlyUserIdScheme - is not implemented correctly. It should in fact check that the path parameter {userId} matches the user id from the OAuth token
It's the same process as the UsersTokenHandler class except checking the userId.~
~AuthenticationResource#createNewAvatar is not implemented as a single transaction. This functionality should be moved into AuthenticationService and properly wrapped with txExpr
The transaction is the JTA transaction or any other transaction?If it's the JTA transaction,we can just move the code from the AuthenticationResource to the AuthenticationService to solve the problem or we need addtional process?~
*~User#getEnabled() returns a string while semantically it is a boolean
Do we need to check the user is enable for every method related with the user?~
~AuthenticationResource#updateUserProfile should be wrapped into a transaction. Consider moving into AuthenticationService~
~AuthenticationResource#getUserAvatar should be wrapped into a transaction. Consider moving into AuthenticationService~
~AuthenticationResource#createNewAvatar should be wrapped into a transaction. Consider moving into AuthenticationService~
~AuthenticationResource#updateUserAvatar should be wrapped into a transaction. Consider moving into AuthenticationService~
~AuthenticationResource#removeUserAvatar should be wrapped into a transaction. Consider moving into AuthenticationService~
~AuthenticationResource - result codes are inconsistent: sometimes result code is 200 for a failed operation, sometimes it is 400 or 500. The general rule for REST operations is that 200 should not be returned for a failed operation
Could you please point the code in the AuthenticationResource ?~
~AuthorizationServlet: in the authorization request https://tools.ietf.org/html/rfc6749#section-4.1.1 the recommended parameter state is missing
The state field should be defined as a constant like wpg/authn ?~
~AuthorizationServlet: the redirect URL to a login page is constructed incorrectly because it doesn't do proper escaping. My recommendation would be to simply store the complete escaped request uri instead of reassembling it with a string substitution (which is not reliable anyway)~
~AuthorizationServlet: in case of error and missing redirectURI the page should not redirect to baidu.com, but rather to a dedicated error page in the authn app which can display the passed error message properly to the user
How to define the error page in the karaf?~
~ChangePasswordServlet - do not expose as a GET method to avoid XSS attacks and exposure of the old password in the URL~
~GetAvatarByUserIdServlet: this is a very strange condition: "null".equals(userId)~
~LoginServlet: should not expose GET handler, only POST~
~LoginServlet: should reply with redirects, not JSON~
~LogoutServlet: instead of forwarding the ServletRequest internally, it makes sense to respond with a 302 redirect so that the user has a clear URL in the browser after logout~
I make it through set the ac_expiration to 0,if the user request the expired code again,invalid_grant would be reponsed~Could you please describe the detail of the applying a block cipher with a pre-configured secret and nonce? The result after encryption is unique too ? Should I configure the aes key in one properties file ?~It's the same process as the UsersTokenHandler class except checking the userId.~The transaction is the JTA transaction or any other transaction?If it's the JTA transaction,we can just move the code from the AuthenticationResource to the AuthenticationService to solve the problem or we need addtional process?~ *~User#getEnabled() returns a string while semantically it is a booleanDo we need to check the user is enable for every method related with the user?~Could you please point the code in the AuthenticationResource ?~stateis missingThe state field should be defined as a constant like wpg/authn ?~How to define the error page in the karaf?~"null".equals(userId)~