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

Remove web.ignoring() for API #6527

Closed deepu105 closed 7 years ago

deepu105 commented 7 years ago

We use the below configuration in our SecurityConfiguration.java

    @Override
    public void configure(WebSecurity web) throws Exception {
        web.ignoring()
            .antMatchers(HttpMethod.OPTIONS, "/**")
            .antMatchers("/app/**/*.{js,html}")
            .antMatchers("/i18n/**")
            .antMatchers("/content/**")
            .antMatchers("/swagger-ui/index.html")
            .antMatchers("/api/register")
            .antMatchers("/api/activate")
            .antMatchers("/api/account/reset-password/init")
            .antMatchers("/api/account/reset-password/finish")
            .antMatchers("/test/**")
            .antMatchers("/h2-console/**");
    }

We have added some API endpoints in there As per the spirng security documenetation below it is not advisable and we already do a permitAll() on those endpoints anyway

Typically the requests that are registered should be that of only static

  • resources. For requests that are dynamic, consider mapping the request to allow all
  • users instead.

@jhipster/developers does anybody know why the above config was added as I remember that we didn't have it some time back

cbornet commented 7 years ago

The permitAll() for oauth2 was added very recently so I think the ignoring() is now not needed anymore

jdubois commented 7 years ago

yes that's very early code I wrote at the beginning of JHipster, this is probably obsolete (but needs to be tested, of course)

jdubois commented 7 years ago

Sorry, I just realize you only speak about the "/api" URLs -> yes that's stupid, they should be removed! Maybe some testing is needed, as I guess the person who put them could have had an issue there.