8000 attachTo for UserSupplier by graziang · Pull Request #48102 · keycloak/keycloak · GitHub
[go: up one dir, main page]

Skip to content

attachTo for UserSupplier - #48102

Closed
graziang wants to merge 1 commit into
keycloak:mainfrom
graziang:issue-48101
Closed

graziang wants to merge 1 commit into
keycloak:mainfrom
graziang:issue-48101

Conversation

@graziang
Copy link
Copy Markdown
Contributor

Closes #48101

Closes keycloak#48101

Signed-off-by: Giuseppe Graziano <g.graziano94@gmail.com>
@graziang
graziang marked this pull request as ready for review May 14, 2026 07:26
@graziang
graziang requested a review from a team as a code owner May 14, 2026 07:26
Comment on lines +24 to +34
@Override
public String getRef(InjectClient annotation) {
String ref = StringUtil.convertEmptyToNull(SupplierHelpers.getAnnotationField(annotation, AnnotationFields.REF));
if (ref != null) {
return ref;
}
// Fallback to attachTo
String attachTo = annotation.attachTo();
return attachTo.isEmpty() ? null : attachTo;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, remove it, after a team discussion, this is not the way we want support current test-framework lifecycle.

Suggested change
@Override
public String getRef(InjectClient annotation) {
String ref = StringUtil.convertEmptyToNull(SupplierHelpers.getAnnotationField(annotation, AnnotationFields.REF));
if (ref != null) {
return ref;
}
// Fallback to attachTo
String attachTo = annotation.attachTo();
return attachTo.isEmpty() ? null : attachTo;
}

@stianst stianst left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced about the attachTo in the first place we added for clients; feels wrong from a test-framework perspective.

Could you elaborate on why this is needed and show some tests where you're using this? My thinking is that it would be better to actually create a realm managed user, or using some of the methods from ManagedRealm.

@lhanusov
Copy link
Copy Markdown
Contributor

this issue won't be implemented, rather we've decided to remove attachTo from client as well

@lhanusov lhanusov closed this May 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

attachTo for UserSupplier

3 participants

0