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

Keycloak docs fix wrong link and add small enhancement #37748

Merged
merged 1 commit into from
Dec 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions docs/src/main/asciidoc/security-keycloak-admin-client.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@

:prerequisites-docker:
include::{includes}/prerequisites.adoc[]
* https://www.keycloak.org/docs/latest/server_installation/index.html[Keycloak]
* https://www.keycloak.org[Keycloak]

== Creating the Project

Check warning on line 25 in docs/src/main/asciidoc/security-keycloak-admin-client.adoc

View workflow job for this annotation

GitHub Actions / Linting with Vale

[vale] reported by reviewdog 🐶 [Quarkus.Headings] Use sentence-style capitalization in 'Creating the Project'. Raw Output: {"message": "[Quarkus.Headings] Use sentence-style capitalization in 'Creating the Project'.", "location": {"path": "docs/src/main/asciidoc/security-keycloak-admin-client.adoc", "range": {"start": {"line": 25, "column": 4}}}, "severity": "INFO"}

First, we need a new project.
Create a new project with the following command:
Expand Down Expand Up @@ -124,8 +124,8 @@
import org.keycloak.admin.client.KeycloakBuilder;
import org.keycloak.representations.idm.RoleRepresentation;

import jakarta.annotations.PostConstruct;
import jakarta.annotations.PreConstruct;
import jakarta.annotation.PostConstruct;
import jakarta.annotation.PreDestroy;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import java.util.List;
Expand Down
6 changes: 3 additions & 3 deletions docs/src/main/asciidoc/security-keycloak-authorization.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
:prerequisites-docker:
include::{includes}/prerequisites.adoc[]
* https://stedolan.github.io/jq/[jq tool]
* https://www.keycloak.org/docs/latest/server_installation/index.html[Keycloak]
* https://www.keycloak.org[Keycloak]

== Architecture

Expand Down Expand Up @@ -115,7 +115,7 @@
import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;

import org.jboss.resteasy.annotations.cache.NoCache;
import org.jboss.resteasy.reactive.NoCache;

import io.quarkus.security.identity.SecurityIdentity;

Expand Down Expand Up @@ -210,9 +210,9 @@
docker run --name keycloak -e KEYCLOAK_ADMIN=admin -e KEYCLOAK_ADMIN_PASSWORD=admin -p 8543:8443 -v "$(pwd)"/config/keycloak-keystore.jks:/etc/keycloak-keystore.jks quay.io/keycloak/keycloak:{keycloak.version} start --hostname-strict=false --https-key-store-file=/etc/keycloak-keystore.jks
----

where `keycloak.version` should be set to `17.0.0` or higher.
where `keycloak.version` should be set to `23.0.0` or higher and the `keycloak-keystore.jks` can be found in https://github.com/quarkusio/quarkus-quickstarts/blob/main/security-keycloak-authorization-quickstart/config/keycloak-keystore.jks[quarkus-quickstarts/security-keycloak-authorization-quickstart/config]
Copy link
Member

@sberyozkin sberyozkin Dec 14, 2023

Choose a reason for hiding this comment

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

Suggested change
where `keycloak.version` should be set to `23.0.0` or higher and the `keycloak-keystore.jks` can be found in https://github.com/quarkusio/quarkus-quickstarts/blob/main/security-keycloak-authorization-quickstart/config/keycloak-keystore.jks[quarkus-quickstarts/security-keycloak-authorization-quickstart/config]
where `keycloak.version` should be set to `23.0.0` or higher.

Users are not expected to use it somehow, the command above picks it up with "$(pwd)"/config/keycloak-keystore.jks, so it is not clear what adding this information helps with, alongside a more important KC version update info

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My point in here is that if user not using quickstart but go through this by him self. He can easily miss this. I can drop it if you stand on this.

Copy link
Member

Choose a reason for hiding this comment

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

@jedla97 Sure, makes sense to keep it, thanks

Copy link
Member

Choose a reason for hiding this comment

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

Please don't use direct links, use variables instead: link:{quickstarts-blob-url}/security-keycloak-authorization-quickstart/config/keycloak-keystore.jks[text]


You should be able to access your Keycloak Server at https://localhost:8543[localhost:8543].

Check warning on line 215 in docs/src/main/asciidoc/security-keycloak-authorization.adoc

View workflow job for this annotation

GitHub Actions / Linting with Vale

[vale] reported by reviewdog 🐶 [Quarkus.TermsSuggestions] Depending on the context, consider using 'because' or 'while' rather than 'as'. Raw Output: {"message": "[Quarkus.TermsSuggestions] Depending on the context, consider using 'because' or 'while' rather than 'as'.", "location": {"path": "docs/src/main/asciidoc/security-keycloak-authorization.adoc", "range": {"start": {"line": 215, "column": 86}}}, "severity": "INFO"}

Log in as the `admin` user to access the Keycloak Administration Console.
Username should be `admin` and password `admin`.
Expand Down