Hi, I've a simple function which tries to enable OIDC authentication. <div class="

<a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/us

ResponseParseError in vaultrs::auth::oidc::role::set about vaultrs HOT 3 CLOSED

chaostheory88 commented on August 24, 2024

ResponseParseError in vaultrs::auth::oidc::role::set

from vaultrs.

Comments (3)

Haennetz commented on August 24, 2024

Hey sorry for the delayed responses.
Can you provide a stack trace? You can enable that feature by setting the env variable RUST_BACKTRACE to 1.
I tried to reproduce it unfortunately I don't have a Google account, so I used a different OIDC Provider to test it.

from vaultrs.

stormshield-gt commented on August 24, 2024

Error("EOF while parsing a value", line: 1, column: 0),

This error means that's the HTTP response had an empty body and we were expecting one. This can happen for methods that can both create and update, like the one you have used.
The empty body case can happen when you are in updating mode but the parameters you provided don't have new information, so there is no real update. Sometimes even if there is an update the API could return with an empty body

In that case we are already ignoring the result (vaultrs::auth::oidc::role::set returns unit ()) changing the implementation of the function from api::exec_with_empty_result to api::exec_with_empty should do the work.

This does raise a bigger concern about how we should handle this create/update function in the library. I've come up with 4 solutions:

catch the error in the calling function and return a clearer error (like i did in #77) . But can catch other errors as well.

Here a sample of the code

 api::exec_with_result(client, endpoint)
            .await
            .map_err(|err| {
                // In the case the response as an empty HTTP Body
                if matches!(
                    err,
                    ClientError::RestClientError {
                        source: rustify::errors::ClientError::ResponseParseError { .. }
                    }
                ) {
                    return ClientError::InvalidUpdateParameter;
                }
                err
            })

systematically return api::exec_with_empty in that case. But sometimes the information returned is useful, so the user would have to issue both a create and then a read. This gets worse when the create method returns an id, where sometimes we have to do a list (to get the id) and then a read.
have an hybrid method execute_with_result_or_empty. I've also experimented this workaround in #77 . It adds an option to the return type, so we are complexifying the most common use cases, for something that is should be rare (update with wrong parameters or an update that return an empty body). As nothing is really specified for the return type of the API, that's maybe the good representation

Here was my working implementation

/// Executes an [Endpoint] and if it returns an empty HTTP response, it returns `None`, otherwise it returns the result.
///
/// This function is mostly useful for endpoints that can both *update* and *create*.
/// The *create* operation will return a result and the *update* one not.
///
/// If there is a result, a few operations are performed on it:
///
/// * Any potential API error responses from the execution are searched for and,
///   if found, converted to a [ClientError::APIError]
/// * All other errors are mapped from [rustify::errors::ClientError] to
///   [ClientError::RestClientError]
/// * An empty content body from the execution is rejected and a
///   [ClientError::ResponseEmptyError] is returned instead
/// * The enclosing [EndpointResult] is stripped off and any warnings found in
///   the result are logged
/// * An empty `data` field in the [EndpointResult] is rejected and a
///   [ClientError::ResponseDataEmptyError] is returned instead
/// * The value from the enclosed `data` field is returned along with any
///   propagated errors.
pub async fn exec_with_result_or_empty<E>(
    client: &impl Client,
    endpoint: E,
) -> Result<Option<E::Response>, ClientError>
where
    E: Endpoint,
{
    info!(
        "Executing {} and expecting maybe a response",
        endpoint.path()
    );
    endpoint
        .with_middleware(client.middle())
        .exec(client.http())
        .await
        .map(|res| {
            if res.response.body().is_empty() {
                Ok(None)
            } else {
                res.wrap::<EndpointResult<_>>()
                    .map_err(ClientError::from)
                    .map(strip)?
                    .map(Some)
                    .ok_or(ClientError::ResponseDataEmptyError)
            }
        })
        .map_err(parse_err)?
}

Separate the create and the update functionality in 2 functions, even if they talk to the same endpoint. The create one will always return data ( api::exec_with_result) and we are could simply ignore the return of the update which is less useful in that case (api::exec_with_empty). The disadvantage is we are not one one with the API but I think it's the more idiomatic

In all cases, we should definitely look how the go implementation (or other SDK) is dealing with such cases

from vaultrs.

Haennetz commented on August 24, 2024

@stormshield-gt thanks for the suggestions and the explanation of the error.

I don't like this one because it blots the calling functions.
I also don't like this option, because the reason you already mention.
If we add a new exec_with_result_or_empty fiction to the API it can be hard to determine when to use exec_with_result and exec_with_result_or_empty. In the vault documentation they don't mention if they return an empty response or not. We should check if we can solve this in the current api::exec_with_result
also cause a lot of bloated code.

The reason this happens in your PR is that when the API call to identity/entity contains an id vault always returns a response with the return code 204 which stands for No Content. Even if you changed something with it.

You can see this in the following examples

curl -v -X POST -H "X-Vault-Request: true" -H "X-Vault-Token: $(vault print token)" -d '{"id":"f79073f1-35b9-a65e-d124-8efc5b0ed160","name":"test4"}' http://127.0.0.1:8200/v1/identity/entity       
Note: Unnecessary use of -X or --request, POST is already inferred.                                                                                                                                       
*   Trying 127.0.0.1:8200...                                                                                                                                                                              
* Connected to 127.0.0.1 (127.0.0.1) port 8200                                                                                                                                                            
> POST /v1/identity/entity HTTP/1.1                                                                                                                                                                       
> Host: 127.0.0.1:8200                                                                                                                                                                                    
> User-Agent: curl/8.6.0                                                                                                                                                                                  
> Accept: */*                                                                                                                                                                                             
> X-Vault-Request: true                                                                                                                                                                                   
> X-Vault-Token: root                                                                                                                                                                                     
> Content-Length: 60                                                                                                                                                                                      
> Content-Type: application/x-www-form-urlencoded                                                   
>                                    
< HTTP/1.1 204 No Content 
< Cache-Control: no-store                    
< Content-Type: application/json                  
< Strict-Transport-Security: max-age=31536000; includeSubDomains                                                                                                                                          
< Date: Sat, 09 Mar 2024 13:58:12 GMT             
<                                                                                                   
* Connection #0 to host 127.0.0.1 left intact

curl -v -X POST -H "X-Vault-Request: true" -H "X-Vault-Token: $(vault print token)" -d '{"id":"f79073f1-35b9-a65e-d124-8efc5b0ed160","name":"test5"}' http://127.0.0.1:8200/v1/identity/entity
Note: Unnecessary use of -X or --request, POST is already inferred.
*   Trying 127.0.0.1:8200...
* Connected to 127.0.0.1 (127.0.0.1) port 8200
> POST /v1/identity/entity HTTP/1.1
> Host: 127.0.0.1:8200
> User-Agent: curl/8.6.0
> Accept: */*
> X-Vault-Request: true
> X-Vault-Token: root
> Content-Length: 60
> Content-Type: application/x-www-form-urlencoded
> 
< HTTP/1.1 204 No Content
< Cache-Control: no-store
< Content-Type: application/json
< Strict-Transport-Security: max-age=31536000; includeSubDomains
< Date: Sat, 09 Mar 2024 13:58:18 GMT
< 
* Connection #0 to host 127.0.0.1 left intact

But Let's move the discussion to about this to you're PR #77

from vaultrs.

ResponseParseError in vaultrs::auth::oidc::role::set about vaultrs HOT 3 CLOSED

Comments (3)

Related Issues (20)

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent