Integrate jwt with mtsj #334

vapadwal · 2020-04-23T11:16:48Z

Few issues still in this PR

currently storing the pkcs file in resources/config is not working security.keystore.location=classpath:config/keystore.pkcs

It works only if we store it to some file location in below format. Here we need to identify some path in mtsj server to store the file.(file:/pathtofile/file.pkcs)

throwing some weird exception

Caused by: java.io.IOException: insufficient data
at java.base/sun.security.util.DerInputBuffer.truncate(DerInputBuffer.java:134) ~[na:na]
at java.base/sun.security.util.DerInputStream.subStream(DerInputStream.java:160) ~[na:na]
at java.base/sun.security.util.DerInputStream.readVector(DerInputStream.java:421) ~[na:na]
at java.base/sun.security.util.DerInputStream.getSequence(DerInputStream.java:337) ~[na:na]
at java.base/sun.security.pkcs.ContentInfo.(ContentInfo.java:132) ~[na:na]
at java.base/sun.security.pkcs.ContentInfo.(ContentInfo.java:109) ~[na:na]
at java.base/sun.security.pkcs12.PKCS12KeyStore.engineLoad(PKCS12KeyStore.java:2007) ~[na:na]
at java.base/sun.security.util.KeyStoreDelegator.engineLoad(KeyStoreDelegator.java:222) ~[na:na]
at java.base/java.security.KeyStore.load(KeyStore.java:1479) ~[na:na]
at com.devonfw.module.security.keystore.common.impl.KeyStoreAccessImpl.getKeyStore(KeyStoreAccessImpl.java:50)

All static secrets were removed from batch

Send data instead of clob

fix logs and avoid one line

fix major bug. Use the equals method instead of ==

devonfw#238)

vapadwal · 2020-04-24T11:08:43Z

Sure but then the stuff does not go to the module but needs to be part of the docker build that combines the WAR with config+keystore.

yes then need to figure out the where to store this file with @dario-rodriguez

hohwille · 2020-04-24T11:10:33Z

Did you put the keystore in core/src/main/resources/config/keystore.pkcs ?
If so the location is wrong:

security.keystore.location=classpath:config/keystore.pkcs

This works for tests inside the core module but in the end the core is bundled to a JAR file that then gets included inside the bootified JAR/WAR of the server module.
Maybe it can work if the keystore is put inside the server module.

vapadwal · 2020-04-24T11:12:05Z

Did you put the keystore in core/src/main/resources/config/keystore.pkcs ?
If so the location is wrong:
security.keystore.location=classpath:config/keystore.pkcs
This works for tests inside the core module but in the end the core is bundled to a JAR file that then gets included inside the bootified JAR/WAR of the server module.
Maybe it can work if the keystore is put inside the server module.

opps this is new to me need to check

vapadwal · 2020-04-27T07:00:25Z

Did you put the keystore in core/src/main/resources/config/keystore.pkcs ?
If so the location is wrong:
security.keystore.location=classpath:config/keystore.pkcs
This works for tests inside the core module but in the end the core is bundled to a JAR file that then gets included inside the bootified JAR/WAR of the server module.
Maybe it can work if the keystore is put inside the server module.
opps this is new to me need to check

Yes now it working when we put the keystore file inside mtsj\server\src\main\resources\keystore.pkcs

Now in local to work we need to run using java -jar mythaistar-bootified.war.

vapadwal · 2020-04-27T07:02:33Z

This PR is for reference only should not be merger now. It still has issue with twofactorAuthentication.
Here now JwtAuthentication works. As it is tightly coupled with twofactorauthentication . twofactorauthentication is ignored

…in devo… (devonfw#321) * Implement example batches with modified devon-batch (see devonfw#190 in devon4j) * Cleanup and documentation. Introduce job parameter for input/output file. * Implemented batch start/stop scheme which works with spring batch alone. No own implementation for launchers are required. * Remove batch disabling in core project * Remove unused super class for tests * Include example batch for restarts * Document start of this batch with H2 auto-server-mode * Fix exit code handling & exiting for web and batch * Implement example for authentication see devonfw/devon4j#90 * just start travis built, no code changes * Provide Javadoc * Upgrade to devon4j 2020.04.001

upgrade spring boot version and devon4j

hohwille

Great update of MTSJ, looks very good in general.
I left some feedback for improvement. Could you check if you can quickly address the suggested improvements and then we can merge this.

hohwille · 2020-04-28T15:41:12Z

angular/src/app/core/interceptor/http-request-interceptor.service.ts

@@ -17,6 +17,7 @@ export class HttpRequestInterceptorService implements HttpInterceptor {
    return this.store.select(fromAuth.getToken).pipe(
      first(),
      flatMap((token) => {
+		token = token.replace('Bearer', '');


check indendation ;)

hohwille · 2020-04-28T15:41:28Z

java/mtsj/core/pom.xml

@@ -63,6 +63,10 @@
      <artifactId>devon4j-jpa</artifactId>
      </dependency> -->

+    <dependency>
+	     <groupId>com.devonfw.java.starters</groupId>


check indendation

hohwille · 2020-04-28T15:42:42Z

java/mtsj/core/pom.xml

    <dependency>
      <groupId>io.jsonwebtoken</groupId>
      <artifactId>jjwt</artifactId>
-      <version>0.7.0</version>
+    </dependency>


shoundn't jjwt now come via devon4j jwt module and MTSJ only use this indirectly so this dependency is not needed anymore?

hohwille · 2020-04-28T15:45:22Z

...e/src/main/java/com/devonfw/application/mtsj/general/common/base/TokenManagementService.java

+
+    UserDetailsClientTo userDetails = new UserDetailsClientTo();
+
+    Authentication auth = this.jwtAuthenticator.authenticate(jwt.replace(JwtConstants.TOKEN_PREFIX, ""));


aint this a duplicated authentication? what is the purpose of this service in MTSJ?
IMHO this is to retrieve UserDetailsClientTo via REST service on the client. If so the existing authentication filter should be used and the UserDetailsClientTo should be created from the spring-security Authentication rather than again authenticating.

hohwille · 2020-04-28T15:46:52Z

...ain/java/com/devonfw/application/mtsj/general/service/impl/rest/SecurityRestServiceImpl.java

@@ -39,7 +43,7 @@
  @Path("/currentuser/")
  public UserDetailsClientTo getCurrentUserDetails(@Context HttpServletRequest request) {

-    return TokenAuthenticationService.getUserdetailsFromToken(request.getHeader("Authorization"));
+    return this.tokenManagementService.getUserdetailsFromTokenJwt(request.getHeader("Authorization"));


exactly what I guesses. IMHO does not make so much sense...
Compare this to the original implementation of this service that should still work even it never knew about JWT.

vapadwal · 2020-04-29T07:58:17Z

I have fix the review comments , but still there is problem with twofactorauthentication and this need some more time to fix

vapadwal · 2020-05-10T15:06:06Z

Fixed 2Fa related error

hohwille

This is good progress on JWT support but IMHO still needs some rework.
@suprishi may you take over and redo with an updated PR replacing this one?

java/mtsj/core/pom.xml

...mtsj/core/src/main/java/com/devonfw/application/mtsj/general/common/base/PreLoginFilter.java

hohwille · 2020-07-27T17:59:54Z

...c/main/java/com/devonfw/application/mtsj/general/common/base/TokenAuthenticationService.java

 public class TokenAuthenticationService {

  /** Logger instance. */
  private static final Logger LOG = LoggerFactory.getLogger(TokenAuthenticationService.class);

+  @Inject
+  private KeyStoreAccess keyStoreAccess;
+
+  @Inject
+  private JwtConfigProperties jwtConfig;
+


I am still wondering why this class is still needed after all.

This class has been used in multiple classes like AdvancedDaoAuthenticationProvider calls method getRolesFromName, PreLoginFilter and TwoFactorFilter uses constant variable OTP. @hohwille

hohwille · 2020-07-27T18:02:31Z

...tsj/core/src/main/java/com/devonfw/application/mtsj/general/common/base/TwoFactorFilter.java

+  private JwtCreator jwtCreator;
+


IMHO 2FA should be independent of or alternative to JWT:

Typically there is a login process for human authentication. For advanced security that process may require a second factor (2FA)

a valid JWT is however a result of a successful authentication and prevents further need for authentication so it replaces the authentication process.

So in the end this still does not make sense to me.

i can see in the PR , it is used to get the token from the Authentication and pass it as header to the HttpServletRequest. So, should i remove this part? @hohwille

markusschuh

IMHO the JWT should be returned to the browser after successful login as part of a JSON structure inside the response body - instead within the "Authorization" header. At least for me this seems to be the most often used method, especially in case of OpenID Connect. I am not aware, there were a standard for this JWT transfer to browser in a pure "JWT auth" architecture, though. So there is not only one right implementation for this.

Nevertheless: The meaning of the Authorization header is not defined, when it is used within the response - so if really a response header should be used for the transfer, that should be better a header name with leading "X-".
Due to that also the return of the "Bearer " prefix (currently within the Authentication header in response to /login) seem superfluous to me. This has no real meaning (since Authorization header in response has no meaning). That might also the reason, the "Bearer" was added again on Angular side (so the resulting header has the "Bearer " twice) - again this addition on browser side is the most often usage for JWT use, I have seen so far in other implementations.

CLAassistant · 2021-01-13T13:26:38Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
5 out of 6 committers have signed the CLA.

✅ sjimenez77
✅ dario-rodriguez
✅ maihacke
✅ vapadwal
✅ derochs
❌ marochs

marochs seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

stale · 2022-09-07T08:26:49Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale · 2022-10-07T09:25:35Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

NaserKa and others added 30 commits June 18, 2019 18:20

Update .gitignore

1b9bbcf

Delete encodings.xml

fa2afa1

Delete misc.xml

11f9e11

Delete modules.xml

b83b421

Delete vcs.xml

5f1363d

Delete workspace.xml

258ae21

Utilize BeanMapper in Usermanagement

b11a60d

Small Fix

5375869

Added dynamic secret generation

33a9766

All static secrets were removed from batch

Merge pull request devonfw#235 from jambulud/develop

75374b8

Send data instead of clob

fix logs and avoid one line

d363c07

fix for lint maxium line lenght of 140

1d6e159

fix after sonar analysis

06c418b

fix strange form to pass integer

ef18635

Merge pull request devonfw#239 from Jorge-Dacal/develop

ec04aec

fix logs and avoid one line

fix major bug. Use the equals method instead of ==

ef2d3d3

Merge pull request devonfw#240 from Jorge-Dacal/develop

a8286c7

fix major bug. Use the equals method instead of ==

fix major bug. Use the equals method instead of ==

6b6e5bc

Merge pull request devonfw#241 from Jorge-Dacal/develop

3cbf805

fix major bug. Use the equals method instead of ==

add configuration for OpenShift

c41687d

add configuration for OpenShift

422dac9

remove healtcheck

60732fb

remove healtcheck

4475b1b

Added 2FA documentation

82e960e

Added link for sidebar

cfdaafe

add configuration for OpenShift angular

3307d5e

devonfw#236: Relocated REST controller interfaces from 'core' to 'api' (

f04d359

devonfw#238)

Merge conflict fix v1

760e628

Merge conflict fix v2

e103b5e

Better explanation added

cf0fed3

update jwt integration with mtsj

edd485a

maihacke and others added 4 commits April 27, 2020 16:46

merge conficts fix

4fb431e

merge conficts fix

f9605b5

Merge pull request devonfw#331 from vapadwal/upgrade_springboot_version

91836f9

upgrade spring boot version and devon4j

hohwille requested changes Apr 28, 2020

View reviewed changes

vapadwal added 2 commits April 29, 2020 11:49

resolve conflicts

a0959cc

updated the review comments

4cce67f

vapadwal added 3 commits April 29, 2020 17:31

updated the lint error

a044717

fix angular issue

a9dab39

updated issues related to 2fa

e5e42fa

vapadwal changed the title ~~WIP:integrate jwt with mtsj~~ Integrate jwt with mtsj May 10, 2020

vapadwal added 2 commits May 10, 2020 20:59

fix linting error

2e2986e

updated application.res

1527894

hohwille requested changes Jul 27, 2020

View reviewed changes

markusschuh reviewed Jul 30, 2020

View reviewed changes

sjimenez77 force-pushed the develop branch 2 times, most recently from c98d581 to 2783216 Compare October 22, 2020 11:36

stale bot added the wontfix label Oct 7, 2022

stale bot closed this Oct 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate jwt with mtsj #334

Integrate jwt with mtsj #334

vapadwal commented Apr 23, 2020 •

edited

Loading

vapadwal commented Apr 24, 2020

hohwille commented Apr 24, 2020

vapadwal commented Apr 24, 2020

vapadwal commented Apr 27, 2020

vapadwal commented Apr 27, 2020

hohwille left a comment

hohwille Apr 28, 2020

hohwille Apr 28, 2020

hohwille Apr 28, 2020

hohwille Apr 28, 2020

hohwille Apr 28, 2020

vapadwal commented Apr 29, 2020

vapadwal commented May 10, 2020

hohwille left a comment

hohwille Jul 27, 2020

ravicm2 Aug 31, 2020 •

edited

Loading

hohwille Jul 27, 2020

ravicm2 Aug 31, 2020

markusschuh left a comment •

edited

Loading

CLAassistant commented Jan 13, 2021 •

edited

Loading

stale bot commented Sep 7, 2022

stale bot commented Oct 7, 2022


		UserDetailsClientTo userDetails = new UserDetailsClientTo();

		Authentication auth = this.jwtAuthenticator.authenticate(jwt.replace(JwtConstants.TOKEN_PREFIX, ""));

Integrate jwt with mtsj #334

Integrate jwt with mtsj #334

Conversation

vapadwal commented Apr 23, 2020 • edited Loading

vapadwal commented Apr 24, 2020

hohwille commented Apr 24, 2020

vapadwal commented Apr 24, 2020

vapadwal commented Apr 27, 2020

vapadwal commented Apr 27, 2020

hohwille left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vapadwal commented Apr 29, 2020

vapadwal commented May 10, 2020

hohwille left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ravicm2 Aug 31, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markusschuh left a comment • edited Loading

Choose a reason for hiding this comment

CLAassistant commented Jan 13, 2021 • edited Loading

stale bot commented Sep 7, 2022

stale bot commented Oct 7, 2022

vapadwal commented Apr 23, 2020 •

edited

Loading

ravicm2 Aug 31, 2020 •

edited

Loading

markusschuh left a comment •

edited

Loading

CLAassistant commented Jan 13, 2021 •

edited

Loading