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-15395. Removed unnecessary Account REST API endpoints #7393

Merged

Conversation

miquelsi
Copy link
Contributor

@miquelsi miquelsi commented Sep 4, 2020

KEYCLOAK-15395. Removed totp/remove (DELETE) and credentials/password (GET, POST) endpoints.

@miquelsi
Copy link
Contributor Author

miquelsi commented Sep 4, 2020

Please @vmuzikar, @ASzc, @abstractj, @slaskawi review at your earliest convenience.

@abstractj
Copy link
Contributor

@ssilvert assuming that this may have an impact in the new account console, would you like to assign someone from your team to review?

@ssilvert
Copy link
Contributor

ssilvert commented Sep 9, 2020

@ssilvert assuming that this may have an impact in the new account console, would you like to assign someone from your team to review?

I took a quick look. As long as the UI tests pass it's OK with me. Kicked off UI test build number 979 for this.

@thomasdarimont
Copy link
Contributor

Removing those endpoints might break existing applications. This should at least be mentioned in the migration guide / changelog.
In general, I think it would be good to have a deprecation process in place where changes like this are announced first (per mailing list / blog, code annotated with @deprecated) before the actual API removal is performed.

@vmuzikar
Copy link
Contributor

vmuzikar commented Sep 9, 2020

@thomasdarimont This is still a preview (unsupported) API. To make it final and supported, we need to do some clean up. ;) I believe it is ok to do this change. @stianst?

@abstractj
Copy link
Contributor

@thomasdarimont This is still a preview (unsupported) API. To make it final and supported, we need to do some clean up. ;) I believe it is ok to do this change. @stianst?

@thomasdarimont thanks for your feedback but I'm kinda of agree with @vmuzikar here. The account REST API is still marked as preview, which means that things may change and will change, until we flip the switch.

@@ -341,116 +341,4 @@ public void setLabel(final @PathParam("credentialId") String credentialId, Strin
// session.userCredentialManager().moveCredentialTo(realm, user, credentialId, newPreviousCredentialId);
// }

Copy link
Contributor

@abstractj abstractj Sep 9, 2020

Choose a reason for hiding this comment

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

We have some unused imports which are safe to remove like:

import org.keycloak.credential.PasswordCredentialProvider;
import org.keycloak.credential.PasswordCredentialProviderFactory;
import org.keycloak.events.EventType;
import javax.ws.rs.POST;

Also, please remove:

private final EventBuilder event;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Thanks, I removed that.

@ssilvert
Copy link
Contributor

@ssilvert assuming that this may have an impact in the new account console, would you like to assign someone from your team to review?

I took a quick look. As long as the UI tests pass it's OK with me. Kicked off UI test build number 979 for this.

The tests on the new account console look fine.

@miquelsi miquelsi force-pushed the msimonma/KEYCLOAK-15395_removed_endpoints branch from 17b915b to 555964b Compare September 10, 2020 06:19
abstractj
abstractj previously approved these changes Sep 10, 2020
Copy link
Contributor

@abstractj abstractj left a comment

Choose a reason for hiding this comment

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

@miquelsi LGTM, will wait until @vmuzikar review so we can merge.

Copy link
Contributor

@vmuzikar vmuzikar left a comment

Choose a reason for hiding this comment

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

Thank you @miquelsi! Changes look good, found one very small nit pick – I think it's safe to merge as is.

public Response removeTOTP() {
auth.require(AccountRoles.MANAGE_ACCOUNT);

session.userCredentialManager().disableCredentialType(realm, user, CredentialModel.OTP);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a very small nit pick.

import org.keycloak.credential.CredentialModel;

is also unused now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Easy to fix, so I removed it :-)

@abstractj abstractj merged commit 2572b14 into keycloak:master Sep 10, 2020
@abstractj
Copy link
Contributor

Merged, thanks @miquelsi

@MatCuk
Copy link

MatCuk commented Nov 2, 2020

Hi,

can any give me more infromation what to use now to change password over REST call?
I'm also asking on discord with not answers :
https://keycloak.discourse.group/t/keycloak-account-api-password-endpoint-removed-why/5274

So my use case is I have my FE app which would call my BE and my BE would forward change password request to keycloak.

@miquelsi miquelsi deleted the msimonma/KEYCLOAK-15395_removed_endpoints branch November 17, 2020 08:49
@loudmouth
Copy link

loudmouth commented Feb 4, 2021

Is there a plan on replacing the functionality to allow end users to update their own passwords as was previously enabled with a POST to the account/credentials/password endpoint?

I know it was in preview, so it is definitely fair game that it was removed, but it was nice to provide our account management experience in our react app with and embedded with other UI relevant to our user experience by relying upon that endpoint. I fully expected as a preview API that it might change, but did not expect that it would simply be removed.

@vmuzikar
Copy link
Contributor

vmuzikar commented Feb 4, 2021

@loudmouth @MatCuk AFAIK there're no plans to support credentials modifications via REST API as it was considered a security risk. An app that is secured by Keycloak should not have access to the user credentials.

As an alternative, you can use Application Initiated Actions (AIA) to trigger a browser flow where user will be prompted to update their password. This is what the new Account Console uses. Another option is to use Admin REST API.

@loudmouth
Copy link

alrighty thank you @vmuzikar

I realize that i may have stumbled upon what is supposed to be a private pull request so I appreciate you taking the time to answer.

It's very nice of you to point me to some documentation as I had been struggling to find the relevant information.

@mapkyca
Copy link

mapkyca commented Feb 21, 2021

Hmm... this is critical functionality for us. Are there plans to build an equivalent, or is the concept just being dropped?

@vmuzikar
Copy link
Contributor

@mapkyca Hi, there are no plans for adding/resetting credentials via REST APi.

@abascivan
Copy link

abascivan commented Mar 25, 2021

@vmuzikar Hi. I tried the example of using AIA that is described on the link you posted but I can't make it work.

If I understand well for example if we want to user to update his profile we should redirect him to an URL that looks like:

http://localhost:8050/auth/realms/my-realm/protocol/openid-connect/auth?response_type=code&client_id=my-
app&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2F&kc_action=update_profile

But When I do so it prompt me the login page then redirect me to the provided redirect_uri. Is the URL that I use seems correct ? Or do I need to install/configure something in order to make it work ?

@sthenault
Copy link

@vmuzikar @abascivan I can't make it work either. Using similar url:

Any clue about this? bug, misconfiguration, bad url?
BTW is there some documentation about AIA beside the md file you mentioned above?

thx!

@vmuzikar
Copy link
Contributor

vmuzikar commented Mar 26, 2021

@abascivan @sthenault I'm afraid it's not that easy to generate the URL "manually". Should be done through the adapter but AFAIK only the JS adapter supports it at the moment. IMHO a contribution for adding AIA support to other adapters would be most welcome! :) You could discuss it on the keycloak-dev list.

Please, if you have any follow-up usage questions, address them to the keycloak-user list for better visibility to other users and team members. Thank you!

@barashnikov
Copy link

barashnikov commented Apr 20, 2021

@vmuzikar Hello, I'm trying to do that via JS adapter but I have no clue to how to do that !

By any chance did you get an example of an implementation for change password ?

Thank you !

@vmuzikar
Copy link
Contributor

@barashnikov I think our Account Console could give you a few hints. ;)

@barashnikov
Copy link

@vmuzikar Thank you for your fast response.

I would like to show you my code :

async componentDidMount() {

if (keycloakEnabled) {
  var keycloak = Keycloak('/keycloak.json');
  await keycloak.init({ onLoad: 'login-required' }).then(authenticated => {
    sessionStorage.setItem('token', keycloak.token);

    keycloak.loadUserInfo().then(function (data) {
      sessionStorage.setItem('Locale', data.locale)
      sessionStorage.setItem('username', data.preferred_username)
      sessionStorage.setItem('name', data.name)
    });


    this.setState({ keycloak: keycloak, authenticated: authenticated, props: keycloak });
    setInterval(() => {
      if (keycloak.isTokenExpired) {
        keycloak.updateToken(-1).then((refresh) => {

          sessionStorage.setItem('token', keycloak.token);
        })

      }
    }, 1000 * 60 * 5);

  });
}

}

As you can see, I'm currently using some methods of keycloak for retrieving profile info for an user.

But I couldn't find anything about change password or maybe I didn't search well.

Thx again !

@vmuzikar
Copy link
Contributor

@barashnikov Please see the link I posted in my previous comment. You basically need to call kecloak.login() with UPDATE_PASSWORD action.

If you have further questions, please use keycloak-user list. An old PR is not the best place to discuss such things. Thank you.

@anwar-bs
Copy link

This is a sample url that prompts for password update UI, tested in 15.0.2

https://your.keycloak.url.com/auth/realms/YOUR_REALM_NAME/protocol/openid-connect/auth?client_id=account-console&redirect_uri=https%3A%2F%2Fgoogle.com&response_type=code&scope=openid&kc_action=UPDATE_PASSWORD&code_challenge=v3edtsDSqpv8xCVtvXU1KxuJZ1soLQp3ZzOz2SyhjMo&code_challenge_method=plain

code_challenge_method which is set in keycloak admin panel for Account-console => settings => Advanced settings => Proof Key for Code Exchange Code Challenge Method must be used (plain or S256)

@saghayar
Copy link

This is a sample url that prompts for password update UI, tested in 15.0.2

https://your.keycloak.url.com/auth/realms/YOUR_REALM_NAME/protocol/openid-connect/auth?client_id=account-console&redirect_uri=https%3A%2F%2Fgoogle.com&response_type=code&scope=openid&kc_action=UPDATE_PASSWORD&code_challenge=v3edtsDSqpv8xCVtvXU1KxuJZ1soLQp3ZzOz2SyhjMo&code_challenge_method=plain

code_challenge_method which is set in keycloak admin panel for Account-console => settings => Advanced settings => Proof Key for Code Exchange Code Challenge Method must be used (plain or S256)

Thank you @anwar-bs , the link you posted worked fine .

1 similar comment
@saghayar
Copy link

This is a sample url that prompts for password update UI, tested in 15.0.2

https://your.keycloak.url.com/auth/realms/YOUR_REALM_NAME/protocol/openid-connect/auth?client_id=account-console&redirect_uri=https%3A%2F%2Fgoogle.com&response_type=code&scope=openid&kc_action=UPDATE_PASSWORD&code_challenge=v3edtsDSqpv8xCVtvXU1KxuJZ1soLQp3ZzOz2SyhjMo&code_challenge_method=plain

code_challenge_method which is set in keycloak admin panel for Account-console => settings => Advanced settings => Proof Key for Code Exchange Code Challenge Method must be used (plain or S256)

Thank you @anwar-bs , the link you posted worked fine .

@hnim1922
Copy link

This is a sample url that prompts for password update UI, tested in 15.0.2

https://your.keycloak.url.com/auth/realms/YOUR_REALM_NAME/protocol/openid-connect/auth?client_id=account-console&redirect_uri=https%3A%2F%2Fgoogle.com&response_type=code&scope=openid&kc_action=UPDATE_PASSWORD&code_challenge=v3edtsDSqpv8xCVtvXU1KxuJZ1soLQp3ZzOz2SyhjMo&code_challenge_method=plain

code_challenge_method which is set in keycloak admin panel for Account-console => settings => Advanced settings => Proof Key for Code Exchange Code Challenge Method must be used (plain or S256)

Can you tell me how can I find the code_challenge before the code_challenge_method?

@IvanDugandzic
Copy link

@loudmouth @MatCuk AFAIK there're no plans to support credentials modifications via REST API as it was considered a security risk. An app that is secured by Keycloak should not have access to the user credentials.

As an alternative, you can use Application Initiated Actions (AIA) to trigger a browser flow where user will be prompted to update their password. This is what the new Account Console uses. Another option is to use Admin REST API.

As i can see in latest Admin API Docs
action PUT /admin/realms/{realm}/users/{id}/reset-password is still available. Can we count on this API in future versions since regarding security it can also be considered as "credentials modifications via REST API" just like actions removed in this PR?

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

Successfully merging this pull request may close these issues.

None yet