-
Notifications
You must be signed in to change notification settings - Fork 96
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
[FEATURE] allow grant type to be specified for client:auth #499
[FEATURE] allow grant type to be specified for client:auth #499
Conversation
@tobiaslohr Could we get this into the next version? It's a pitfall many folks run into when using sfcc-ci with a dw.json file present in the same dir. |
Any chance of getting this fix merged? |
…sword credentials when dw.json is present
1bdf738
to
bc7bc83
Compare
@tobiaslohr I just updated the branch with the latest from master, any chance we can get this merged now? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good stuff!
Been a while since I've checked in on this. @tobiaslohr mentioned on Slack that anyone with admin rights can merge PRs - can someone else maybe get this merged in and get a release out with this fix? |
Looks like this is still coming up from other users as an issue in Slack: @tobiaslohr how do we get @sandragolden or @clavery to be admins on this project if your other work priorities aren't letting you maintain this project anymore? SFCC-CI is a tool that many rely on and it seems as thought Salesforce has simply abandoned its users at this point. |
Hi @sandragolden, thanks a lot for contributing! We can merge this. However, the way how we approach this is not ideal. It should not be up to the user of the CLI to tell it which grant type to use. This should be up to the tool to determine. For this particular case, I suggest we simply stop using the user credentials from the dw.json to make a token request with Reason being is, that neither AM nor any Commerce application require the use of this grant type any longer. It's also discouraged for security reasons. My recommendation is we merge the PR, but we adjust this to (remove the new argument |
This sounds great to me, many thanks for reviewing and adjusting @tobiaslohr |
For the records: by changing the working directory which does not contain the dw.json and execute the command from there. Then it does not pick up the user credentials from the dw.json, as it's not finding it any longer. We are not following my previous suggestion at #499 (comment) in order to build this in a backwards compatible manner as this had been requested by folks. In a next major version we will remove the Resource Owner Password Credentials grant entirely. |
Closes #200 |
closes #245
I do not see this as a breaking change because everything will continue to work normally but we have a new option that will allow you to force client_credentials grant type
Related to Resource Owner Password Credentials Authentication Failure
What PR adjusts:
Allows grant type to be specified for
client:auth
. For example:sfcc-ci client:auth -t client_credentials
. Default grant type will remain unchanged and is set topassword
by default.Why?
I'm running a local project where I'm running into the
Error: Authentication failed: Resource owner authentication failed
mentioned in the related doc above. Because I have adw.json
file in my project, the grant type ofpassword
is always being used. Due to my Account Manager setup, this is throwing an error. If I force the grant type ofclient_credentials
, everything works as expected because I've added the necessary permissions to my client.