MarcGiffing / wicket-spring-boot

Spring Boot starter for Apache Wicket
150 stars 61 forks source link

Switching to Google Oauth2 as authentication provider doesn't work properly #176

Open abiuan opened 3 years ago

abiuan commented 3 years ago

Hello, I'm developing a wicket web application that needs authentication. I'm using spring security for that. Using an InMemoryUserDetailsManager works all fine. Switching to OAuth2 authentication doesn't work. The following exception: IllegalStateException: Couln't find sign in page - please annotate the sign in page with @com.giffing.wicket.spring.boot.context.scan.WicketSignInPage means that wicket is looking for a login page therefore it is unable to read authentication information from spring security context, I suppose.

I wrote a simple spring boot application with security, but without wicket-spring-boot, and It works, so I suppose is something related to wicket-spring-boot. Does it need a specific configuration for OAuth2? What I'm doing wrong?

Best regards

My security environment is quite simple:

I'm using @EnableAutoConfiguration on @SpringBootApplication class. My application.yml is

security:
    oauth2:
      client:
        registration:
          google:
            clientId: myclentId
            clientSecret: myclientSecret
            scope:
              - email
              - profile

my WebSecurityConfigurerAdapter is

@EnableWebSecurity
public class HttpSecurityConfig {

    @Configuration
    public static class OAuth2WebSecurityConfigurerAdapter extends WebSecurityConfigurerAdapter {

        @Override
        protected void configure(HttpSecurity http) throws Exception {
            http.authorizeRequests()
                .anyRequest().authenticated()
                .and()
                .oauth2Login()
            ;
        }

        @Bean(name="authenticationManager")
        @Override
        public AuthenticationManager authenticationManagerBean() throws Exception {
            return super.authenticationManagerBean();
        }
    }
}
MarcGiffing commented 3 years ago

Can you provide an complete example? Can you provide your wicket pages? Can you explain the login/authenticiation process on your page?

MarcGiffing commented 3 years ago

I'm not familiar with tje Spring Boot Oauth confugiration. Can you mark your landing page with the WicketSignIn annotation? If you already logged in on startup then it ahould ve enough. What should happen if the credentials are incorrect in the properties?

abiuan commented 3 years ago

Thanks Marc,

I did more investigations with a fresh demo wicket application, built starting from wicket "Quick Start Wizard", adding spring-boot dependencies.

In this application I do not have a login page. I'm using the defaults of spring security that builds login and login?logout pages (same page with different parameters). I'm trying this because google oauth2 autentication uses its login form, I do not need mine. All the application has to be authenticated, as you can see in the configure() method of WebSecurityConfigurerAdapter.

If I don't use @AuthorizeInstantiation annotation everything works fine. I can login with spring security form and view my homepage (the only page in this simple application). If I go to "/logout" url, I correctly logout from my application and redirected by application to the login form.

Adding @AuthorizeInstantiation("USER") annotation the behaviour changes.

It seems that wicket-spring-boot needs WicketSignInPage annotation. The page ("/login") exists but is not created by wicket. The same for "/logout" of course.

Looking at wicket documentation I see that AuthenticatedWebApplication.getSignInPageClass() accept only a Page.class argument, therefore it seems it's not possible to have an external login page. Is it true?

Then I also tried to add a custom login page using a wicket WebPage. I changed the security configuration as:

.authorizeRequests()
    .antMatchers("/login/**").permitAll()
    .anyRequest().authenticated()
    .and()
    .formLogin().defaultSuccessUrl("/homepage")
        .usernameParameter("username")
        .passwordParameter("password")
            .loginPage("/login").permitAll()
    ;

In this case I'm unable to access my application homepage. After authentication, my custom login page is shown again without any error on stdout or browser developer console. But in this case I suppose there is something wrong in HttpSecurity configuration (after days of readings, spring-security still remains quite obscure...).

Anny suggest?

Best regards

My application

This is my configuration:

WickeApplication

@SpringBootApplication
public class WicketApplication
{
    public static void main(String[] args) throws Exception {
        new SpringApplicationBuilder()
            .sources(WicketApplication.class)
            .run(args);
    }

}

InitConfiguration

@ApplicationInitExtension
public class InitConfiguration implements WicketApplicationInitConfiguration {

    @Override
    public void init(WebApplication webApplication) {
        webApplication.getCspSettings().blocking().disabled();
    }

}

HomePage

@WicketHomePage
@AuthorizeInstantiation("USER")
public class HomePage extends WebPage {
    private static final long serialVersionUID = 1L;

    public HomePage(final PageParameters parameters) {
        super(parameters);

        add(new Label("version", getApplication().getFrameworkSettings().getVersion()));

    }
}

CustomSecurityConfig

@EnableWebSecurity
public class CustomSecurityConfig {

    @Bean
    public BCryptPasswordEncoder passwordEncoder() {
        return new BCryptPasswordEncoder();
    }

    @Bean
    public UserDetailsService userDetailsService() {

        InMemoryUserDetailsManager manager = new InMemoryUserDetailsManager();
        manager.createUser(
                User.withUsername("admin")
                 .password(passwordEncoder().encode("admin"))
                 .authorities("USER", "ADMIN")
                 .build());
        manager.createUser(
                User.withUsername("user")
                 .password(passwordEncoder().encode("user"))
                 .authorities("USER")
                 .build());
        return manager;
    }

    @Configuration
    public class ClientSecurityConfig extends WebSecurityConfigurerAdapter {

        @Override
        protected void configure(HttpSecurity http) throws Exception {

            http.authorizeRequests()
                        .antMatchers("/login").permitAll()
                        .anyRequest().authenticated()
                        .and()
                        . formLogin().permitAll()
                    ;
        }

        @Bean(name="authenticationManager")
        @Override
        public AuthenticationManager authenticationManagerBean() throws Exception {
            return super.authenticationManagerBean();
        }

    }

}

pom.xml dependencies

    <dependencies>
        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-starter-security</artifactId>
        </dependency>
        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-actuator</artifactId>
        </dependency>
        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-configuration-processor</artifactId>
        </dependency>
        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-devtools</artifactId>
            <scope>runtime</scope>
            <optional>true</optional>
        </dependency>
        <dependency>
            <groupId>org.springframework.boot</groupId>
            <artifactId>spring-boot-starter-test</artifactId>
            <scope>test</scope>
            <exclusions>
                <exclusion>
                    <groupId>org.junit.vintage</groupId>
                    <artifactId>junit-vintage-engine</artifactId>
                </exclusion>
            </exclusions>
        </dependency>
        <dependency>
            <groupId>org.springframework.security</groupId>
            <artifactId>spring-security-test</artifactId>
            <scope>test</scope>
        </dependency>
        <dependency>
            <groupId>com.giffing.wicket.spring.boot.starter</groupId>
            <artifactId>wicket-spring-boot-starter</artifactId>
            <version>${wicket-spring-boot-starter.version}</version>
        </dependency>

        <!-- LOGGING DEPENDENCIES - SLF4J-SIMPLE -->
        <dependency>
            <groupId>org.slf4j</groupId>
            <artifactId>slf4j-simple</artifactId>
        </dependency>

        <!--  JUNIT DEPENDENCY FOR TESTING -->
        <dependency>
            <groupId>org.junit.jupiter</groupId>
            <artifactId>junit-jupiter-engine</artifactId>
            <scope>test</scope>
        </dependency>

    </dependencies>
MarcGiffing commented 3 years ago

I think the problem is that you're logged in in Spring Security but a signedIn Flag of Wicket is missing. See org.apache.wicket.authroles.authentication.AuthenticatedWebSession. Take a look at the example project on the login page.

Wicket is calling the LoginPage if the user is not loggedin: See AuthenticatedWebApplication

@Override
public final void onUnauthorizedInstantiation(final Component component)
{
    if (component instanceof Page)
    {
        if (!AbstractAuthenticatedWebSession.get().isSignedIn())
        {
            // Redirect to intercept page to let the user sign in
            restartResponseAtSignInPage();

Can you please extend from the class com.giffing.wicket.spring.boot.starter.configuration.extensions.external.spring.security.SecureWebSession and overwrite the method isSignedIn() and check it the with authentication authentication = SecurityContextHolder.getContext().getAuthentication(); . https://www.baeldung.com/get-user-in-spring-security

You can register your custom SecureWebSession with the following snippet:

@Bean
@Primary
public AuthenticatedWebSessionConfig authenticatedWebSessionConfig(){
    return new AuthenticatedWebSessionConfig() {

        @Override
        public Class<? extends AbstractAuthenticatedWebSession> getAuthenticatedWebSessionClass() {
            return AbiuanSecureWebSession.class;
        }
    };
}
abiuan commented 3 years ago

Unfortunately AuthenticatedWebSession.isSignedIn() and .signIn are final. I checked the SecurityContextHolder in the constructor of my homepage and it has authentication info with the correct username and roles.

I also modified SecureWebSession class, since I compiled wicket-spring-boot from source and authentication info are present too.

$ git diff --ignore-space-at-eol ./src/main/java/com/giffing/wicket/spring/boot/starter/configuration/extensions/external/spring/security/SecureWebSession.java | fromdos
diff --git a/wicket-spring-boot-starter/src/main/java/com/giffing/wicket/spring/boot/starter/configuration/extensions/external/spring/security/SecureWebSession.java b/wicket-spring-boot-starter/src/main/java/com/giffing/wicket/spring/boot/starter/configuration/extensions/external/spring/security/SecureWebSession.java
index 720817d..66a220f 100644
--- a/wicket-spring-boot-starter/src/main/java/com/giffing/wicket/spring/boot/starter/configuration/extensions/external/spring/security/SecureWebSession.java
+++ b/wicket-spring-boot-starter/src/main/java/com/giffing/wicket/spring/boot/starter/configuration/extensions/external/spring/security/SecureWebSession.java
@@ -16,6 +16,8 @@ import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.security.web.context.HttpSessionSecurityContextRepository;

+import java.util.stream.Collectors;
+
 /**
  * Spring Security Implementation of Wickets {@link AuthenticatedWebSession}. 
  * 
@@ -60,6 +62,7 @@ public class SecureWebSession extends AuthenticatedWebSession implements Seriali
    @Override
    public Roles getRoles() {
        Roles roles = new Roles();
+       printAuthenticationInfo();
        if (isSignedIn()) {
            Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
            for (GrantedAuthority authority : authentication.getAuthorities()) {
@@ -69,4 +72,12 @@ public class SecureWebSession extends AuthenticatedWebSession implements Seriali
        return roles;
    }

+   private void printAuthenticationInfo() {
+       Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
+
+       System.out.println("User: " + authentication.getName() + ", Roles: " + authentication.getAuthorities().stream().map(GrantedAuthority::toString).collect(Collectors.joining(", ")));
+
+   }
+
+
 }

Best regards

MarcGiffing commented 3 years ago

I checked the SecurityContextHolder in the constructor of my homepage and it has authentication info with the correct username and roles. Yes but did you also check the isSignedIn() method?

Instead of extending the SecureWebSession you have to extend from AbstractAuthenticatedWebSession to get rid of the final method.

abiuan commented 3 years ago

Hello, I created a custom AuthenticatedWebSession as you suggested, grabbing code from org.apache.wicket.authroles.authentication.AuthenticatedWebSession and com.giffing.wicket.spring.boot.starter.configuration.extensions.external.spring.security.SecureWebSession sources. I added a debug statement for every method. As you can see the only methods called are getRoles() and isSignedIn().

... .CustomSecureWebSession    : getRoles()
... .CustomSecureWebSession    : isSignedIn(): false
... .CustomSecureWebSession    : isSignedIn(): false

CustomSecureWebSession.java

public class CustomSecureWebSession extends AbstractAuthenticatedWebSession implements Serializable {

    private Logger logger = LoggerFactory.getLogger(getClass());

    private static final long serialVersionUID = 1L;

    private static final String SPRING_SECURITY_CONTEXT_KEY = HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY;

    @SpringBean(name = "authenticationManager")
    private AuthenticationManager authenticationManager;

    private final AtomicBoolean signedIn = new AtomicBoolean(false);

    /**
     * @param request
     */
    public CustomSecureWebSession(Request request) {
        super(request);
        Injector.get().inject(this);
    }

    protected boolean authenticate(String username, String password) {

        logger.info("authenticate(). username: {}, password: {}", username, password);

        try {
            Authentication auth = authenticationManager.authenticate(
                new UsernamePasswordAuthenticationToken(username, password));
            if (auth.isAuthenticated()) {
                SecurityContextHolder.getContext().setAuthentication(auth);
                WebSession httpSession = WebSession.get();
                if(httpSession != null) {
                    httpSession.setAttribute(SPRING_SECURITY_CONTEXT_KEY, SecurityContextHolder.getContext());
                }
                return true;
            }
            return false;
        } catch (AuthenticationException e) {
            return false;
        }
    }

    @Override
    public Roles getRoles() {

        logger.info("getRoles()");

        Roles roles = new Roles();
        if (isSignedIn()) {
            Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
            for (GrantedAuthority authority : authentication.getAuthorities()) {
                roles.add(authority.getAuthority());
            }
        }
        return roles;
    }

    public final boolean signIn(final String username, final String password)
    {
            boolean authenticated = authenticate(username, password);

            logger.info("signIn(). username: {}, password: {}, authenticated: {}", username, password, authenticated);

            if (!authenticated && signedIn.get())
            {
                    signOut();
            }
            else if (authenticated && signedIn.compareAndSet(false, true))
            {
                    bind();
            }
            return authenticated;
    }

    protected final void signIn(boolean value)
    {
        logger.info("signIn(): {}", value);
        signedIn.set(value);
    }

    /**
     * @return true, if user is signed in
     */
    @Override
    public final boolean isSignedIn()
    {
        logger.info("isSignedIn(): {}", signedIn.get());
        return signedIn.get();
    }

    /**
     * Sign the user out.
     * <p>This method is an alias of {@link #invalidate()}</p>
     */
    public void signOut()
    {
        logger.info("signOut()");
        invalidate();
    }

    /**
     * Call signOut() and remove the logon data from where ever they have been persisted (e.g.
     * Cookies)
     */
    @Override
    public void invalidate()
    {
        logger.info("invalidate()");
        signedIn.set(false);
        super.invalidate();
    }
}
MarcGiffing commented 3 years ago

You must get rid of the signedIn variable. Only implement from AbstractAuthenticatedWebSession. Use the implementation from SecureWebSession and implement the isSignedIn() method with the help of SecurityContextHolder.getContext().getAuthentication();.

abiuan commented 3 years ago

Hello, following your suggest I created the ExternalSecureWebSession class extending AbstractAuthenticatedWebSession and registered in the WebSecurityConfigurerAdapter. Now it works fine.

You helped me a lot. Many thanks.

public class ExternalSecureWebSession extends AbstractAuthenticatedWebSession implements Serializable {

    private static final long serialVersionUID = 1L;

    /**
     * @param request
     */
    public ExternalSecureWebSession(Request request) {
        super(request);
    }

    @Override
    public Roles getRoles() {
        Roles roles = new Roles();
        if (isSignedIn()) {
            Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
            for (GrantedAuthority authority : authentication.getAuthorities()) {
                roles.add(authority.getAuthority());
            }
        }
        return roles;
    }

    @Override
    public boolean isSignedIn() {
        return SecurityContextHolder.getContext().getAuthentication().isAuthenticated();
    }
}
MarcGiffing commented 3 years ago

Maybe this should be the default implementation in the this Wicket Starter Project. I've to check it so please let the ticket open.