43 add security compatibility#57

Open

Fayeredd wants to merge 22 commits into1701-jan09-java:masterfrom

Fayeredd:43-add-security-compatibility

Contributor

Fayeredd commented Mar 29, 2017

Security compatibility using JWT.

Temporary token generator/validator is JwtTokenUtil.

Security classes (by package):
com.revature.config: WebSecurityConfig

com.revature.repositories: UserRepository

com.revature.model.security: Authority, AuthorityName, User

com.revature.security: JwtAuthenticationEntryPoint, JwtAuthenticationFilter, JwtAuthenticationRequest, JwtAuthenticationSuccessHandler, JwtTokenUtil (TEMP), JwtUser, JwtUserFactory

com.revature.security.controllers: AuthenticationRestController, UserRestController

com.revature.security.exceptions: JwtTokenMissingException

com.revature.security.service: JwtAuthenticationResponse, JwtUserDetailsServiceImpl

Fayeredd added 11 commits

March 28, 2017 15:22


          Add security compatibility, temporary token generator; testing needed.

01065f8


          update for readability and demo token generator

10595c1


          debugging

2809d71


          bugfix

ff159cb


          security tested, operational

7df2a9d


          Add security compatibility, temporary token generator; testing needed.

70d54ad


          update for readability and demo token generator

f5b81d0


          debugging

d6c09b9


          bugfix

c9770fe


          security tested, operational

cb0606c


          Merge branch '43-add-security-compatibility' of https://github.com/Fa…

3d97a42

…yeredd/interview-evaluations into 43-add-security-compatibility

octocat-jedi reviewed

View reviewed changes

src/main/java/com/revature/InterviewEvaluationsApplication.java

		@@ -11,6 +11,7 @@
		public class InterviewEvaluationsApplication {

octocat-jedi Mar 29, 2017

Add a private constructor to hide the implicit public one.

Fayeredd added 3 commits

March 30, 2017 09:32


          code clean

cb4a191


          bugfix

5825af0


          code clean

76d4c6f

Contributor

cmatheny commented Mar 30, 2017

@tjkemper please review this.

Contributor

tjkemper commented Mar 30, 2017

This is a lot in one PR.

Initial reactions

The Swagger endpoint /swagger-ui.html should load without a JWT in the request
Each request must have a JWT signed with some secret: dog
- The API doesn't handle passwords or generate tokens

Contributor Author

Fayeredd commented Mar 30, 2017

Claims have a secret: mySecret (see application.yml and JwtTokenUtil)
This secret is validated through the JwtAuthenticationFilter.
The password/token generation is handled by JwtTokenUtil, a test class placed in the structure to test the rest of it. It can be replaced later with an external generator.

Fayeredd added 2 commits

March 31, 2017 09:37


          refactored security into core and extra; core to remain with the API,…

8569e90

… extra to be discarded when external token generation/validation is implemented


          refactored

673e6c5

octocat-jedi reviewed

View reviewed changes

src/main/java/com/revature/security/extra/service/JwtUserDetailsServiceImpl.java

+                  private UserRepository userRepository;
+                  @Override
+                  public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException {

octocat-jedi Mar 31, 2017

Remove the declaration of thrown exception 'org.springframework.security.core.userdetails.UsernameNotFoundException' which is a runtime exception.

octocat-jedi reviewed

View reviewed changes

src/main/java/com/revature/security/extra/JwtUser.java

+                  private final boolean enabled;
+                  private final Date lastPasswordResetDate;
+                  public JwtUser(

octocat-jedi Mar 31, 2017

Constructor has 9 parameters, which is greater than 7 authorized.

octocat-jedi reviewed

View reviewed changes

src/main/java/com/revature/security/extra/JwtTokenUtil.java

+                      try {
+                          final Claims claims = getClaimsFromToken(token);
+                          audience = (String) claims.get(CLAIM_KEY_AUDIENCE);
+                      } catch (Exception e) {

octocat-jedi Mar 31, 2017

Either log or rethrow this exception.

octocat-jedi reviewed

View reviewed changes

src/main/java/com/revature/security/extra/JwtTokenUtil.java

+                      Date expires;
+                      try {
+                          final Claims claims = getClaimsFromToken(token);
+                          expires = claims.getExpiration();

octocat-jedi Mar 31, 2017

NullPointerException might be thrown as 'claims' is nullable here

octocat-jedi reviewed

View reviewed changes

src/main/java/com/revature/security/extra/JwtTokenUtil.java

+                      try {
+                          final Claims claims = getClaimsFromToken(token);
+                          expires = claims.getExpiration();
+                      } catch (Exception e) {

octocat-jedi Mar 31, 2017

Either log or rethrow this exception.

octocat-jedi reviewed

View reviewed changes

src/main/java/com/revature/security/extra/JwtTokenUtil.java

+                      Date created;
+                      try {
+                          final Claims claims = getClaimsFromToken(token);
+                          created = new Date((Long) claims.get(CLAIM_KEY_CREATED));

octocat-jedi Mar 31, 2017

NullPointerException might be thrown as 'claims' is nullable here

octocat-jedi reviewed

View reviewed changes

src/main/java/com/revature/security/extra/JwtTokenUtil.java

+                      try {
+                          final Claims claims = getClaimsFromToken(token);
+                          created = new Date((Long) claims.get(CLAIM_KEY_CREATED));
+                      } catch (Exception e) {

octocat-jedi Mar 31, 2017

Either log or rethrow this exception.

octocat-jedi reviewed

View reviewed changes

src/main/java/com/revature/security/extra/JwtTokenUtil.java

+                      String refreshedToken;
+                      try {
+                          final Claims claims = getClaimsFromToken(token);
+                          claims.put(CLAIM_KEY_CREATED, new Date());

octocat-jedi Mar 31, 2017

NullPointerException might be thrown as 'claims' is nullable here

octocat-jedi reviewed

View reviewed changes

src/main/java/com/revature/security/extra/JwtTokenUtil.java

+                                  .setSigningKey(secret)
+                                  .parseClaimsJws(token)
+                                  .getBody();
+                      } catch (ExpiredJwtException | MalformedJwtException | SignatureException | UnsupportedJwtException | IllegalArgumentException e) {

octocat-jedi Mar 31, 2017

Either log or rethrow this exception.

octocat-jedi reviewed

View reviewed changes

src/main/java/com/revature/security/extra/JwtTokenUtil.java

+                          final Claims claims = getClaimsFromToken(token);
+                          claims.put(CLAIM_KEY_CREATED, new Date());
+                          refreshedToken = generateToken(claims);
+                      } catch (Exception e) {

octocat-jedi Mar 31, 2017

Either log or rethrow this exception.

octocat-jedi reviewed

View reviewed changes

src/main/java/com/revature/security/extra/JwtTokenUtil.java

+                      String username;
+                      try {
+                          final Claims claims = getClaimsFromToken(token);
+                          username = claims.getSubject();

octocat-jedi Mar 31, 2017

NullPointerException might be thrown as 'claims' is nullable here

octocat-jedi reviewed

View reviewed changes

src/main/java/com/revature/security/extra/JwtTokenUtil.java

+                      try {
+                          final Claims claims = getClaimsFromToken(token);
+                          username = claims.getSubject();
+                      } catch (Exception e) {

octocat-jedi Mar 31, 2017

Either log or rethrow this exception.

octocat-jedi reviewed

View reviewed changes

src/main/java/com/revature/security/extra/JwtTokenUtil.java

+                      String audience;
+                      try {
+                          final Claims claims = getClaimsFromToken(token);
+                          audience = (String) claims.get(CLAIM_KEY_AUDIENCE);

octocat-jedi Mar 31, 2017

NullPointerException might be thrown as 'claims' is nullable here

tjkemper requested changes

View reviewed changes

Contributor

tjkemper left a comment

Overall good progress.

Focus on verifying the JWT has the correct signature.

The API does not need to worry about usernames/passwords or generating tokens.

src/main/java/com/revature/security/controllers/AuthenticationRestController.java

+                                  authReq.getPassword())
+                          );
+                      SecurityContextHolder.getContext().setAuthentication(authentication);

Contributor

tjkemper Apr 1, 2017

Let's avoid using SecurityContext. We want our API to be stateless.

src/main/java/com/revature/security/core/JwtAuthenticationFilter.java

+                  private String tokenHeader;
+                  @Override
+                  protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws ServletException, IOException {

Contributor

tjkemper Apr 1, 2017

You're on the right track here.

May want something like:

try {

    Jwts.parser().setSigningKey(key).parseClaimsJws(compactJws);

    //OK, we can trust this JWT

} catch (SignatureException e) {

    //don't trust the JWT!
}

Notice we do not know anything about the user. We only care that the JWT was signed with our secret and it wasn't tampered with.


          Remove unneeded classes; move login, token generation, token authenti…

4a3ab96

…cation to new project

Contributor Author

Fayeredd commented Apr 12, 2017 •

edited

Loading

Build fails for non-existent dependency (JwtAuthenticationFilter [8,37])

The dependency is a local second project. The dependency is in the POM and the directory is in the classpath. Any ideas as to why it can't find this dependency would be appreciated.

POM:

<dependency>
<groupId>com.revature</groupId>
<artifactId>RevatureSecurityLogin</artifactId>
<version>1.0-SNAPSHOT</version>
<scope>compile</scope>
</dependency>

Classpath:
<classpathentry kind="var" path="M2_REPO/com/revature/RevatureSecurityLogin/1.0-SNAPSHOT/RevatureSecurityLogin-1.0-SNAPSHOT.jar"/>

Fayeredd added 3 commits

April 12, 2017 15:57


          fix dependency compatibility

dca0587


          Fully Functional. All files included under any package named 'extra' …

9ef22cb

…are to be removed and placed in a seperate Authorization Server service. All files in the 'static' folder likewise belong to the future Authorization Server service


          Merge branch 'master' into 43-add-security-compatibility

a938d0d

octocat-jedi reviewed

View reviewed changes

src/main/java/com/revature/security/extra/controllers/UserRestController.java

+                      String token = req.getHeader(tokenHeader);
+                      String username = jwtTokenUtil.getUsernameFromToken(token);
+                      JwtUser user = (JwtUser) userDetailsService.loadUserByUsername(username);

octocat-jedi Apr 14, 2017

Immediately return this expression instead of assigning it to the temporary variable "user".

octocat-jedi reviewed

View reviewed changes

src/main/java/com/revature/security/extra/controllers/AuthenticationRestController.java

+                  //This value can be found in the application.yml file
+                  @RequestMapping(value = "${jwt.route.authentication.path}", method = RequestMethod.POST)
+                  public ResponseEntity<?> createAuthenticationToken(@RequestBody JwtAuthenticationRequest authReq, Device dev) throws AuthenticationException{

octocat-jedi Apr 14, 2017

Remove usage of generic wildcard type.
Remove the declaration of thrown exception 'org.springframework.security.core.AuthenticationException' which is a runtime exception.

octocat-jedi reviewed

View reviewed changes

src/main/java/com/revature/security/extra/controllers/AuthenticationRestController.java

+                  //This value can be found in the application.yml file
+                  @RequestMapping(value = "${jwt.route.authentication.refresh}", method = RequestMethod.GET)
+                  public ResponseEntity<?> refreshAndGetAuthenticationToken(HttpServletRequest req){

octocat-jedi Apr 14, 2017

Remove usage of generic wildcard type.

Fayeredd added 2 commits

April 17, 2017 09:40


          reference fix

69623dc


          Merge branch '43-add-security-compatibility' of https://github.com/Fa…

bba4301

…yeredd/interview-evaluations into 43-add-security-compatibility

octocat-jedi commented Apr 17, 2017

SonarQube analysis reported 18 issues

15 major
3 minor

Watch the comments in this conversation to review them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet