fix: make input HTML IDs unique in authorization-popup.jsx

[sethidden]

The HTML id= params "client_id" and "client_secret" applied to inputs in the autorization modal can become duplicated. HTML IDs are supposed to be unique within the document. This breaks password manager (1Password at least) prefills, as it gets confused about which field is which

Description

Before you write me off as some HTML spec purist, the duplicate IDs actually break password managers.
There's a feature in 1Password where you can create a custom field with the same name as an input's ID
Like this (custom_id, client_secret, oauth_username, oauth_password):

Before the PR

Now, you may try and happily go to the Swagger authorization-popup.jsx view and try to prefill your fancy new entry and hope it matches the values to inputs correctly, but you will be met with this:

Unfortunately, as you can see above, 1Password prefilled the first 3 fields correctly, but failed to fill the client_secret field.

You can go here to inspect the Swagger html yourself: https://spartacus-demo.eastus.cloudapp.azure.com:8443/occ/v2/swagger-ui/index.html

First client_secret input: (the one we wanted to be autofilled by 1Passowrd but wasn't)

Second client_secret input: (the one that was wrongly autofilled by 1password)

As you can see, the IDs are exactly the same, which is why 1Password gets confused.

---

After the PR

With my PR, the inputs between sections have the _{flow} suffix, so they're unique. I added them for both the client_id and client_secret, even though only client_secret was problematic:

With my PR, we can see that 1Password prefills as expected
First set (the one that previously didn't prefill but now prefills fine):

and its complimentary "client_id" field:

Second filed (doesn't prefill now):

Naturally because the IDs changed I needed to change the 1Password field names as well:

Motivation and Context

To improve prefilling Authorization fields with password managers and improve HTML spec compliance.

How Has This Been Tested?

Included in Description section.

Screenshots (if appropriate):

Included in previous sections.

Checklist

My PR contains...

• No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
• Dependency changes (any modification to dependencies in package.json)
• Bug fixes (non-breaking change which fixes an issue)
• Improvements (misc. changes to existing features)
• Features (non-breaking change which adds functionality)

My changes...

• are breaking changes to a public API (config options, System API, major UI change, etc).
• are breaking changes to a private API (Redux, component props, utility functions, etc.).
• are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
• are not breaking changes.

Documentation

• My changes do not require a change to the project documentation.
• My changes require a change to the project documentation.
• If yes to above: I have updated the documentation accordingly.

Automated tests

• My changes can not or do not need to be tested.
• My changes can and should be tested by unit and/or integration tests.
• If yes to above: I have added tests to cover my changes.
• If yes to above: I have taken care to cover edge cases in my tests.
• All new and existing tests passed.

[sethidden]

You can test this yourself by going to http://localhost:3200/ and putting https://spartacus-demo.eastus.cloudapp.azure.com:8443/occ/v2/api-docs in the bar at the top, then going to Authorize

[char0n]

LGTM! Thanks for contributing

[char0n]

@sethidden you'd have to address the e2e tests before we can merge this.

[sethidden]

Sure, I'll take a look at them within the next few days. Thank you for taking a look

…

On November 20, 2023 1:05:45 PM GMT+01:00, "Vladimír Gorej" ***@***.***> wrote: @sethidden you'd have to address the e2e tests before we can merge this. -- Reply to this email directly or view it on GitHub: #9390 (comment) You are receiving this because you were mentioned. Message ID: ***@***.***>

[sethidden]

@char0n I adjusted both the E2E suites that were failing in the commit 0747e97

Can you re-run the workflows prosim 🇨🇿 ? They need maintainer approval.

Shame I didn't contribute earlier, I was in Prague for a month till yesterday so I could've done it all in Cafedu haha

[char0n]

Changes look fine! Thanks

[char0n]

Can you re-run the workflows prosim 🇨🇿 ? They need maintainer approval.

Done ;]