Skip to content

An example about how to enable policy enforcement to Elytron/Jakarta applications#396

Merged
mposolda merged 1 commit intokeycloak:mainfrom
pedroigor:issue-19540-quickstart
Apr 19, 2023
Merged

An example about how to enable policy enforcement to Elytron/Jakarta applications#396
mposolda merged 1 commit intokeycloak:mainfrom
pedroigor:issue-19540-quickstart

Conversation

@pedroigor
Copy link
Copy Markdown
Contributor

@pedroigor pedroigor commented Apr 12, 2023

  • Migrating the app-authz-jee-servlet quickstart to Jakarta
  • Minor updates to jakarta profile to allow running integration tests using Wildfly

@pedroigor pedroigor changed the title Issue 19540 quickstart An example about how to enable policy enforcement to Elytron/Jakarta applications Apr 12, 2023
@pedroigor pedroigor marked this pull request as draft April 12, 2023 15:23
@pedroigor pedroigor force-pushed the issue-19540-quickstart branch 7 times, most recently from 1113cb2 to ef6b657 Compare April 12, 2023 23:05
@pedroigor pedroigor marked this pull request as ready for review April 12, 2023 23:10
@pedroigor pedroigor force-pushed the issue-19540-quickstart branch 2 times, most recently from c2283df to 9d81807 Compare April 13, 2023 22:15
Copy link
Copy Markdown
Contributor

@mposolda mposolda left a comment

Choose a reason for hiding this comment

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

@pedroigor Added few inline comments. Marking this as "request changes" until we figure how to tackle this.

Comment thread app-authz-jakarta-servlet/config/realm-import.json Outdated
<scope>compile</scope>
<groupId>org.keycloak</groupId>
<artifactId>keycloak-admin-client-jakarta</artifactId>
<version>21.0.2</version>
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 guess the hardcoded version here should be avoided?

Is it possible to use <version>${version.keycloak}</version> ? Or ideally avoid <version> element entirely and make sure that correct version is just taken from the parent pom (keycloak BOM) as I've mentioned in the other comments to this file?

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.

Created follow-up #402 for this.

Comment thread app-authz-jakarta-servlet/pom.xml Outdated
Comment thread app-authz-jakarta-servlet/pom.xml Outdated
Comment thread app-authz-jakarta-servlet/src/test/resources/quickstart-realm.json Outdated
@pedroigor pedroigor force-pushed the issue-19540-quickstart branch 2 times, most recently from 2a20ad5 to ad120ae Compare April 14, 2023 15:10
@pedroigor
Copy link
Copy Markdown
Contributor Author

@mposolda About the keycloak-admin-jakarta version. I'm not sure why but GHA fails because it can't find the snapshot version.

I'll see why.

@pedroigor pedroigor force-pushed the issue-19540-quickstart branch from ad120ae to 6ce70ac Compare April 14, 2023 15:32
Copy link
Copy Markdown
Contributor

@mposolda mposolda left a comment

Choose a reason for hiding this comment

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

@pedroigor Still few inline questions added

Comment thread app-authz-jakarta-servlet/config/realm-import.json Outdated
Comment thread app-authz-jakarta-servlet/src/test/resources/quickstart-realm.json Outdated
@pedroigor pedroigor force-pushed the issue-19540-quickstart branch from 6ce70ac to 1865901 Compare April 17, 2023 11:40
@pedroigor
Copy link
Copy Markdown
Contributor Author

Hope is fine now.

@pedroigor pedroigor force-pushed the issue-19540-quickstart branch from 1865901 to a30d72a Compare April 18, 2023 14:31
@pedroigor pedroigor force-pushed the issue-19540-quickstart branch from a30d72a to 895de6b Compare April 19, 2023 11:01
Copy link
Copy Markdown
Contributor

@mposolda mposolda left a comment

Choose a reason for hiding this comment

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

@pedroigor Thanks for the updates!

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.

2 participants