jhipster / generator-jhipster

JHipster is a development platform to quickly generate, develop, & deploy modern web applications & microservice architectures.
https://www.jhipster.tech
Apache License 2.0
21.54k stars 4.02k forks source link

Refactor to remove Spring Security deprecation warnings on startup #18534

Closed mraible closed 1 year ago

mraible commented 2 years ago
*Overview of the issue

I created a new app with the main branch using the following .yo-rc.json:

{
  "generator-jhipster": {
    "applicationType": "monolith",
    "authenticationType": "jwt",
    "baseName": "TwentyOnePoints",
    "blueprints": [],
    "buildTool": "gradle",
    "cacheProvider": "ehcache",
    "clientFramework": "angularX",
    "clientPackageManager": "npm",
    "clientTheme": "none",
    "clientThemeVariant": "",
    "creationTimestamp": 1651363424551,
    "cypressCoverage": false,
    "databaseType": "sql",
    "devDatabaseType": "h2Disk",
    "devServerPort": 4200,
    "dtoSuffix": "DTO",
    "enableGradleEnterprise": false,
    "enableHibernateCache": true,
    "enableSwaggerCodegen": false,
    "enableTranslation": true,
    "entitySuffix": "",
    "jhiPrefix": "jhi",
    "jhipsterVersion": "7.8.1",
    "jwtSecretKey": "NWRjNTgyZDc4NmM3Y2E3ZGU2MmNiMGU1ODg1YjI3Y2U2NDE1MDQyM2M3MGIzOTYzMWI0NGJhNTViNzNmYWE4ZTE5NTQyYzBiNTAyYjFkMmJkMDUwY2Y5OTllOGQ0ZGYxZWIzNTA5Y2FhN2UwMjRiMzEyZmYzYTRiNGZjMGY1MWM=",
    "languages": ["en", "fr"],
    "messageBroker": false,
    "nativeLanguage": "en",
    "otherModules": [],
    "packageName": "org.jhipster.health",
    "pages": [],
    "prodDatabaseType": "postgresql",
    "reactive": false,
    "searchEngine": "elasticsearch",
    "serverPort": "8080",
    "serverSideOptions": ["searchEngine:elasticsearch"],
    "serviceDiscoveryType": "no",
    "skipCheckLengthOfIdentifier": false,
    "skipFakeData": false,
    "skipUserManagement": false,
    "testFrameworks": ["cypress"],
    "websocket": false,
    "withAdminUi": true
  }
}

On startup, it gives me a warning about the Spring Security configuration:

2022-05-01 08:37:27.196  WARN 37119 --- [  restartedMain] o.s.s.c.a.web.builders.WebSecurity       
 : You are asking Spring Security to ignore Ant [pattern='/**', OPTIONS]. This is not recommended -- 
 please use permitAll via HttpSecurity#authorizeHttpRequests instead.
2022-05-01 08:37:27.196  WARN 37119 --- [  restartedMain] o.s.s.c.a.web.builders.WebSecurity       
 : You are asking Spring Security to ignore Ant [pattern='/app/**/*.{js,html}']. This is not recommended -- 
 please use permitAll via HttpSecurity#authorizeHttpRequests instead.
2022-05-01 08:37:27.196  WARN 37119 --- [  restartedMain] o.s.s.c.a.web.builders.WebSecurity       
 : You are asking Spring Security to ignore Ant [pattern='/i18n/**']. This is not recommended -- 
 please use permitAll via HttpSecurity#authorizeHttpRequests instead.
2022-05-01 08:37:27.196  WARN 37119 --- [  restartedMain] o.s.s.c.a.web.builders.WebSecurity       
 : You are asking Spring Security to ignore Ant [pattern='/content/**']. This is not recommended -- 
 please use permitAll via HttpSecurity#authorizeHttpRequests instead.
2022-05-01 08:37:27.196  WARN 37119 --- [  restartedMain] o.s.s.c.a.web.builders.WebSecurity       
 : You are asking Spring Security to ignore Ant [pattern='/h2-console/**']. This is not recommended -- 
 please use permitAll via HttpSecurity#authorizeHttpRequests instead.
2022-05-01 08:37:27.196  WARN 37119 --- [  restartedMain] o.s.s.c.a.web.builders.WebSecurity       
 : You are asking Spring Security to ignore Ant [pattern='/swagger-ui/**']. This is not recommended -- 
 please use permitAll via HttpSecurity#authorizeHttpRequests instead.
2022-05-01 08:37:27.196  WARN 37119 --- [  restartedMain] o.s.s.c.a.web.builders.WebSecurity       
 : You are asking Spring Security to ignore Ant [pattern='/test/**']. This is not recommended 
 -- please use permitAll via HttpSecurity#authorizeHttpRequests instead.

If I change the .yo-rc.json to have `"reactive": true and disable caching:

"reactive": true,
"cacheProvider": "no",
"enableHibernateCache": false,

There are no warnings from Spring Security. There is this warning though:

2022-05-01 09:09:30.583  WARN 39645 --- [  restartedMain] o.s.data.convert.CustomConversions       
 : Registering converter from class java.util.BitSet to class java.lang.Boolean as reading converter 
 although it doesn't convert from a store-supported type! You might want to check your annotation 
 setup at the converter implementation.
Motivation for or Use Case

Security-related issues shouldn't be ignored.

Reproduce the error

Create a new app with the main branch using the .yo-rc.json from above.

Related issues

https://spring.io/blog/2022/02/21/spring-security-without-the-websecurityconfigureradapter

atomfrede commented 2 years ago

I have tried adding the ignored pattern to normal security config with permitall. It works well, except some inteference with the default x frame option deny (e.g. swagger api spec or h2 console). We might set it to same-origin or keep it with deny and use the csp frame-ancestor with self.

Not sure to be honest whats the best way. The csp is supported my all all major browser we officially support.

vishal423 commented 2 years ago

I also noticed the same issue, and I believe the out-of-box x-frame-options default with SAMEORIGIN should suffice for monolithic applications. In the case of micro-services, it may not work as the origin will change (registry, control-centre) and thus require more control with frame-ancestor and that seems not well supported in the Firefox.

atomfrede commented 2 years ago

At least with frame-ancestor self I didn't notice any issues with firefox. But I like your proposal to differentiate between monolith and microservice. Will see to provide a PR for that

vishal423 commented 2 years ago

Reactive implementation seems no different and uses pattern exclusion to bypass security. We need to be consistent in our strategy and align both tracks to use the same approach.

atomfrede commented 2 years ago

Will align both implementations of course :)

pmverma commented 2 years ago

Just for future reference, moving ignore pattern to permitAll has some performance impact as mentioned here and it will be fixed by https://github.com/spring-projects/spring-security/issues/10913

atomfrede commented 2 years ago

@mshima I guess this is done now (via https://github.com/jhipster/generator-jhipster/pull/19184 and https://github.com/jhipster/generator-jhipster/pull/19205)? If so let's close this and please claim the bounty.

mshima commented 2 years ago

@atomfrede new need to check if reactive implementation matches imperative.

mshima commented 11 months ago

@DanielFran bug bounty claimed https://opencollective.com/generator-jhipster/expenses/172339

DanielFran commented 11 months ago

@mshima approved