Skip to content

Conversation

@Abdulbois
Copy link

@Abdulbois Abdulbois commented Aug 28, 2025

- Update approach of retrieving auth request url while building authorization request

Signed-off-by: Abdulbois <[email protected]>
@Abdulbois
Copy link
Author

Abdulbois commented Aug 28, 2025

@sbihel, please review.
As described in #55, these lines cause an error when the other field is None while building the WalletMetadata because the UntypedObject map will not contain the authorization_endpoint field.

@cobward
Copy link
Collaborator

cobward commented Aug 28, 2025

Can you provide a minimal example showing the issue this is resolving?

@Abdulbois
Copy link
Author

Abdulbois commented Aug 28, 2025

Can you provide a minimal example showing the issue this is resolving?

Just for testing purposes, in order to bypass build_with_session_id validation, try to parse authorization_endpoint while calling build method of RequestBuilder

    pub async fn build(self, wallet_metadata: WalletMetadata) -> Result<(Uuid, Url)> {
        let uuid = Uuid::new_v4();
        let authorization_endpoint = wallet_metadata // Test parsing
            .get::<AuthorizationEndpoint>()
            .parsing_error()?;

        let authorization_request_url = self.build_with_session_id(uuid, wallet_metadata).await?;

        Ok((uuid, authorization_request_url))
    }

@cobward
Copy link
Collaborator

cobward commented Aug 28, 2025

I see, this change is fine, but it doesn't actually address the problem in #55. I think WalletMetadata needs a rethink.

@cobward
Copy link
Collaborator

cobward commented Aug 28, 2025

Actually I'm concerned that this could be a break some implementations that edit the inner UntypedObject, as now the request will be built with the original AuthorizationEndpoint even if the inner value was updated.

@Abdulbois
Copy link
Author

Abdulbois commented Aug 28, 2025

I see, this change is fine, but it doesn't actually address the problem in #55. I think WalletMetadata needs a rethink.

Seems, current changes will at least fix that error and remove that workaround described in #55

Totally agree with the point that other field of WalletMetadata should be handled correctly

@Abdulbois
Copy link
Author

Actually I'm concerned that this could be a break some implementations that edit the inner UntypedObject, as now the request will be built with the original AuthorizationEndpoint even if the inner value was updated.

If I understood correctly, currently inner methods of WalletMetadata doesn't touch/update value of authorization_endpoint while using inner(UntypedObject) field, is not it?
Or you mean something else?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WalletMetadata:new() leads to error with AuthorizationEndpoint

2 participants