Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verifying a new user account does not unlock that account. #108

Open
spierepf opened this issue Nov 29, 2018 · 6 comments
Open

Verifying a new user account does not unlock that account. #108

spierepf opened this issue Nov 29, 2018 · 6 comments

Comments

@spierepf
Copy link

Steps to Reproduce

I've created a demo grails app that uses spring-security-ui and contains a failing integration test that I would expect to pass.

$ grails test-app

Expected Behaviour

The test creates a User object with a registration token. That User object has the accountLocked field set to true (which corresponds to the state of that field when a new user account is created through the browser).

I would expect that when the user clicks on the verification link that they receive by email (/registration/verifyRegistration?t=xxxxxxxxxxx) that the accountLocked field would be set to false.

Actual Behaviour

The user is logged in, but their account is still locked, so that future login attempts will fail.

Environment Information

I'm on Windows 10, but I've also tried it on Ubuntu 16.04

$ grails --version
|Grails Version: 3.3.8
|Groovy Version: 2.4.15
|JVM Version: 1.8.0_161

Example Application

https://github.com/spierepf/demo

@spierepf spierepf changed the title Should this integration test succeed, or am I missing something? Verifying a new user account does not unlock that account. Nov 29, 2018
@spierepf
Copy link
Author

spierepf commented Nov 29, 2018

For quick reference, here is my integration test.

package demo

import grails.plugins.rest.client.RestResponse
import grails.plugins.rest.client.RestBuilder
import grails.testing.mixin.integration.Integration
import grails.transaction.*
import spock.lang.Shared
import spock.lang.Specification

import grails.plugin.springsecurity.ui.RegistrationCode

@Integration
@Rollback
class VerifyRegistrationSpec extends Specification {

    @Shared RestBuilder rest = new RestBuilder()

    def setup() {
    }

    def cleanup() {
    }

    void "test something"() {
    	given:"a user account is created and given a registration code"
            def user = new User(username:"username", password:"password", accountLocked:true, accountExpired:false, enabled:true).save()
            def registrationCode = new RegistrationCode(username:user.username).save()

        when:"that user engages the verify registration action with their registration code's token"
            RestResponse resp = rest.get("http://localhost:${serverPort}/verifyRegistration?t=${registrationCode.token}")

        then:"that user account should be unlocked, not expired, and enabled"
            resp.status == 200
            def updatedUser = User.findByUsername(user.username)
            updatedUser.accountLocked == false
            updatedUser.accountExpired == false
            updatedUser.enabled == true
    }
}

@spierepf
Copy link
Author

Interestingly, the test passes if I call uiRegistrationCodeStrategy.finishRegistration explicitly:

    void "test something"() {
        given:"a user account is created and assigned a registration code"
            def user = new User(username:"username", password:"password", accountLocked:true, accountExpired:false, enabled:true).save()
            def registrationCode = new RegistrationCode(username:user.username).save()

        when:"the uiRegistrationCodeStrategy is asked to finishRegistration explicitly"
            uiRegistrationCodeStrategy.finishRegistration(registrationCode)

        then:"that user account should be unlocked, not expired, and enabled"
            def updatedUser = User.findByUsername(user.username)
            updatedUser.accountLocked == false
            updatedUser.accountExpired == false
            updatedUser.enabled == true
    }

@ddelponte ddelponte self-assigned this Apr 8, 2019
@ddelponte
Copy link
Collaborator

The good news is that the code which unlocks a user's account upon successful verification is working properly. I believe the issue here is specific to the use of an integration test to test controllers.

According to the documentation:

To integration test controllers it is recommended you use create-functional-test command to create a Geb functional test. See the following section on functional testing for more information.

More information is available at https://docs.grails.org/3.3.8/guide/testing.html#functionalTesting

Does your test work if you convert it to a functional test?

Also, please note that the url utilized by the RestBuilder in your test was incorrect.

You are using:

RestResponse resp = rest.get("http://localhost:${serverPort}/verifyRegistration?t=${registrationCode.token}")

but it should be

RestResponse resp = rest.get("http://localhost:${serverPort}/register/verifyRegistration?t=${registrationCode.token}")

@spierepf
Copy link
Author

spierepf commented Apr 9, 2019

I've updated the test in my demo project, but I get the same result. I changed both the url as you suggested, and turned the integration test into a functional one.

@ddelponte
Copy link
Collaborator

I've created a PR at #112 that will resolve this issue.

I've requested that @sdelamo review it. He may be able to provide additional insight or a better approach, but I've confirmed my changes do fix the issue in your demo app.

@tadeubas
Copy link
Contributor

tadeubas commented Mar 8, 2021

@ddelponte and @spierepf I've noticed the same problem on a new application that used the plugin. The problem was that I haven't create in my app the necessary default ROLE_USER as defined in DefaultUiSecurityConfig.groovy ``register.defaultRoleNames = ['ROLE_USER']. Because of this an exception is thrown and the user remains locked. I've seen the integration test don't create this ROLE too, that might be the problem

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

No branches or pull requests

3 participants