Copilot commented on code in PR #1207:
URL:
https://github.com/apache/grails-spring-security/pull/1207#discussion_r2817896004
##########
plugin-core/docs/src/docs/tutorials/usingControllerAnnotations.adoc:
##########
@@ -430,6 +432,49 @@ Some things to note about the preceding `BootStrap.groovy`:
* The example does not use a traditional GORM many-to-many mapping for the
User pass:[<==>] Role relationship; instead you are mapping the join
table with the `UserRole` class. This performance optimization helps
significantly when many users have one or more common roles.
* We explicitly flush (using `withSession`) because `BootStrap` does not run
in a transaction or OpenSessionInView.
+* This example works correctly because the `UserPasswordEncoderListener` (a
Spring-managed bean) handles password encoding during the GORM
`PreInsertEvent`. If you are using the older domain class pattern with
`beforeInsert()` instead, see the note below.
+
+[[bootstrapPasswordEncoding]]
+===== Password Encoding in BootStrap
+
+If you are using the older User domain class pattern where
`springSecurityService` is declared as a `transient` field and password
encoding happens in `beforeInsert()`, passwords will **not** be encoded when
creating users in `BootStrap.groovy`. This is because domain class instances
are plain Groovy objects - their transient service references are not autowired
by Spring at construction time. The `springSecurityService` field will be
`null`, and the safe-navigation operator (`?.`) in `encodePassword()` will
silently skip encoding.
+
+To work around this, inject the `passwordEncoder` bean directly in `BootStrap`
and pre-encode passwords before saving:
+
+[source,groovy]
+.`BootStrap.groovy`
+----
+import grails.gorm.transactions.Transactional
+import org.springframework.security.crypto.password.PasswordEncoder
+
+class BootStrap {
+
+ PasswordEncoder passwordEncoder // <1>
+
+ def init = {
+ addTestUser()
+ }
+
+ @Transactional
+ void addTestUser() {
+ def adminRole = new Role(authority: 'ROLE_ADMIN').save()
+
+ String encodedPassword = passwordEncoder.encode('password') // <2>
+ def testUser = new User(username: 'me', password:
encodedPassword).save()
+
+ UserRole.create testUser, adminRole
+
+ UserRole.withSession {
+ it.flush()
+ it.clear()
+ }
+ }
+}
+----
+<1> The `PasswordEncoder` bean is auto-configured by the plugin and can be
injected into `BootStrap` by name.
+<2> Pre-encode the password before constructing the User instance. This
bypasses the `beforeInsert()` hook entirely.
+
+TIP: If you are using the `beforeInsert()` pattern, you must also guard
against double-encoding. Since the password is already encoded before `save()`,
the `beforeInsert()` callback will attempt to encode it again. Either check
whether the password is already encoded, or remove the `beforeInsert()` hook
and always encode explicitly. The recommended approach is to use the
`UserPasswordEncoderListener` pattern shown above, which avoids this issue
entirely.
Review Comment:
This TIP states that with the `beforeInsert()` pattern the callback will
“attempt to encode it again” after pre-encoding, but earlier in this section
you explain that `springSecurityService` is `null` for newly constructed
instances in `BootStrap.groovy`, so the `beforeInsert()` encoding logic would
no-op (no double-encoding). Consider rephrasing to clarify the specific
scenarios where double-encoding can happen (e.g., if a Spring-managed listener
encodes passwords, or if the domain instance does have an injected
`springSecurityService`).
```suggestion
TIP: Be careful not to double-encode passwords if you also use a
`beforeInsert()`-style encoding hook or a Spring-managed listener (for example,
a `User` domain class with an injected `springSecurityService`, or a
`UserPasswordEncoderListener` that encodes on insert/update). In those cases,
the callback or listener would see the already encoded value produced in
`BootStrap` and attempt to encode it again. Ensure that only one mechanism is
responsible for encoding (for example, rely solely on a listener such as
`UserPasswordEncoderListener`, or disable automatic encoding for users created
in `BootStrap`).
```
##########
plugin-core/docs/src/docs/tutorials/usingControllerAnnotations.adoc:
##########
@@ -430,6 +432,49 @@ Some things to note about the preceding `BootStrap.groovy`:
* The example does not use a traditional GORM many-to-many mapping for the
User pass:[<==>] Role relationship; instead you are mapping the join
table with the `UserRole` class. This performance optimization helps
significantly when many users have one or more common roles.
* We explicitly flush (using `withSession`) because `BootStrap` does not run
in a transaction or OpenSessionInView.
+* This example works correctly because the `UserPasswordEncoderListener` (a
Spring-managed bean) handles password encoding during the GORM
`PreInsertEvent`. If you are using the older domain class pattern with
`beforeInsert()` instead, see the note below.
+
+[[bootstrapPasswordEncoding]]
+===== Password Encoding in BootStrap
+
+If you are using the older User domain class pattern where
`springSecurityService` is declared as a `transient` field and password
encoding happens in `beforeInsert()`, passwords will **not** be encoded when
creating users in `BootStrap.groovy`. This is because domain class instances
are plain Groovy objects - their transient service references are not autowired
by Spring at construction time. The `springSecurityService` field will be
`null`, and the safe-navigation operator (`?.`) in `encodePassword()` will
silently skip encoding.
+
+To work around this, inject the `passwordEncoder` bean directly in `BootStrap`
and pre-encode passwords before saving:
+
+[source,groovy]
+.`BootStrap.groovy`
+----
+import grails.gorm.transactions.Transactional
+import org.springframework.security.crypto.password.PasswordEncoder
+
+class BootStrap {
+
+ PasswordEncoder passwordEncoder // <1>
+
+ def init = {
+ addTestUser()
+ }
+
+ @Transactional
+ void addTestUser() {
+ def adminRole = new Role(authority: 'ROLE_ADMIN').save()
+
+ String encodedPassword = passwordEncoder.encode('password') // <2>
+ def testUser = new User(username: 'me', password:
encodedPassword).save()
+
+ UserRole.create testUser, adminRole
+
+ UserRole.withSession {
+ it.flush()
+ it.clear()
+ }
+ }
+}
+----
+<1> The `PasswordEncoder` bean is auto-configured by the plugin and can be
injected into `BootStrap` by name.
+<2> Pre-encode the password before constructing the User instance. This
bypasses the `beforeInsert()` hook entirely.
Review Comment:
The note for <2> says pre-encoding “bypasses the `beforeInsert()` hook
entirely”, but `beforeInsert()` will still run; pre-encoding just avoids
relying on `springSecurityService` being injected to perform encoding. Please
reword this to avoid implying the GORM callback is skipped.
```suggestion
<2> Pre-encode the password before constructing the `User` instance so that
password encoding is handled explicitly here, rather than relying on a
`beforeInsert()` hook or an injected `springSecurityService` in the domain
class.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
