#12379 Update redirect URIs
Closed: Fixed a month ago by zlopez. Opened 2 months ago by gundersanne.

NOTE

If your issue is for security or deals with sensitive info please
mark it as private using the checkbox below.

Describe what you would like us to do:


Verify, and if need be update, the redirect URIs of the image builder consolehrc-cli OIDC client.

For stg:
[https://console.stg.foo.fedorainfracloud.org:1337/*, https://console.stg.fedorainfracloud.org/*]

For production:
[https://console.fedorainfracloud.org/*]

When do you need this to be done by?


Whenever is convenient.


Metadata Update from @zlopez:
- Issue priority set to: Waiting on Assignee (was: Needs Review)
- Issue tagged with: low-gain, low-trouble, ops

2 months ago

Metadata Update from @zlopez:
- Issue assigned to zlopez

2 months ago

I don't see any redirect containing these URIs in our proxies playbook. Could you specify what are the URIs you want to redirect from?

Oh i thought when setting up OIDC clients (https://pagure.io/fedora-infrastructure/issue/12223) you had to enter valid redirect urls? I just wanted to make sure that the above urls are present.

Sorry, I didn't realized you are asking about OIDC entry. The redirect URI for consolerhc-cli is now set to urn:ietf:wg:oauth:2.0:oob for production and to urn:ietf:wg:oauth:2.0:oob for staging. But I also see consolerhc, which has https://console.stg.fedorainfracloud.org/authorize set for redirect URI for staging and https://console.fedorainfracloud.org/authorize for production.

Is this correct or should I do some changes?

Hm can we have multiple valid redirect URIs? If so let's keep urn:ietf:wg:oauth:2.0:oob as well? Or is there only a single redirect uri possible?

For consolerhc the redirect uri shouldn't change, only for consolerh-cli.

Yes, there could be multiple redirect URIs. Let me add the staging one first so we can test it.

I did the update, but when running the playbook I encountered the following issue:

TASK [base : Drop in a little system_identification note] **********************************************************************************************************************************************************************************
Tuesday 28 January 2025  12:43:07 +0000 (0:00:05.627)       0:00:34.898 ******* 
Tuesday 28 January 2025  12:43:07 +0000 (0:00:05.627)       0:00:34.897 ******* 
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: ansible.errors.AnsibleUndefinedVariable: 'csi_security_category' is undefined. 'csi_security_category' is undefined
fatal: [ipsilon01.stg.iad2.fedoraproject.org]: FAILED! => {"changed": false, "msg": "AnsibleUndefinedVariable: 'csi_security_category' is undefined. 'csi_security_category' is undefined"}

I looked at the variable and it's really undefined for anything except logdetective02, logdetective01 and bodhi-backend01. I assume some recent commit did something it shouldn't. I'm just not sure where exactly the change happened.

Found the commit, it's removing plenty of group_vars from ansible https://pagure.io/fedora-infra/ansible/c/b3d6a90b9a49d1e8c8a0399fa491609f3b2a9bff?branch=main

@kevin I'm not sure why this commit is removing CSI related vars. Do you know the reason, or it was just something missed that wasn't cleaned as part of this commit?

@gundersanne Sorry for the inconvenience, but this needs to be solved first as it's blocking the playbook from running. I will update the ticket when the staging is deployed for testing.

Found the commit, it's removing plenty of group_vars from ansible https://pagure.io/fedora-infra/ansible/c/b3d6a90b9a49d1e8c8a0399fa491609f3b2a9bff?branch=main

@kevin I'm not sure why this commit is removing CSI related vars. Do you know the reason, or it was just something missed that wasn't cleaned as part of this commit?

CSI is... no more. You can't even find a copy of it online outside of poking around on archive.org.
We shouldn't be refering to a thing that you can't refer to anymore and isn't maintained or looked at by anyone.

This came out of noticing our /etc/motd was updated from the hosts repo (which we don't update anymore) and referred to CSI.

So, I thought the best thing was to just drop mentions to it and do our own thing.

I have removed that system_identification call... the motd one we added is the new thing. That should get things working for now?

Thanks @kevin, that was exactly what I wanted.

@gundersanne The change is now deployed in staging, could you test if everything is working as expected. We will continue with production after that.

I assume the issue is with the * at the end of redirect URI. I wasn't sure if that would work.

Hi @zlopez, thank you for looking at this.

We initially requested to add * at the end of the redirect URIs to allow nested paths, such as https://console.stg.fedorainfracloud.org/image-builder, without needing to explicitly list each possible path. However, it seems that Fedora’s OIDC provider may not support wildcard URIs in the redirect_uri field.

To avoid issues, let’s try registering the base URLs without * and confirm whether nested paths still work as expected. If needed, we can explicitly add additional URIs later.

I removed * from the URIs and deployed the change on staging, you can try it now.

Thanks for your help! The redirect URI now works as expected.
However, we’re facing an issue during the token exchange. After a successful login, our SPA makes a request to the token endpoint https://id.stg.fedoraproject.org/openidc/Token
with the following data:

{
    "grant_type": "authorization_code",
    "redirect_uri": "https://console.stg.fedorainfracloud.org/",
    "code": "<CODE>",
    "client_id": "consolerhc-cli"
}

but it returns a 400 error:

{
    "error": "invalid_client",
    "error_description": "client authentication error"
}

I've also tried enabling PKCE and passing code_verifier, but I still get the same result.

Does consolerhc-cli need additional configuration to support public clients (SPA without client secret)?
Is there a setting that requires client authentication even though we’re using PKCE?
Appreciate your help!

Does the authentication work with the original redirect URI urn:ietf:wg:oauth:2.0:oob?

I can see this error in Ipsilon logs
Client authentication with invalid auth method: none

Thanks for checking the logs
Since this is a public SPA, it should be treated as a public client and not require client_secret, instead relying on PKCE.
Could you check if the client can be updated to allow none as a valid authentication method?

@kevin @abompard Do you know if that is possible?

Yeah it's possible, we already do that for public clients such as the FMN frontend (in javascript), the client_id is fmn-frontend and it has token_endpoint_auth_method="none"

So it's just token_endpoint_auth_method, let me adjust that.

@amirfefer The change is now deployed, let's try it again.

I did the necessary changes for production and deployed them. @amirfefer Could you try if everything works?

Let me close this as fixed. Feel free to reopen if there is still something we missed.

Metadata Update from @zlopez:
- Issue close_status updated to: Fixed
- Issue status updated to: Closed (was: Open)

a month ago

Log in to comment on this ticket.

Metadata
Boards 1
ops Status: Backlog