Skip to content
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

Server segfaults on parse_url #139

Closed
korydraughn opened this issue Nov 4, 2023 · 4 comments
Closed

Server segfaults on parse_url #139

korydraughn opened this issue Nov 4, 2023 · 4 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@korydraughn
Copy link
Contributor

Bug Report

I attempted to reach the /authenticate endpoint by typing the URL in Google Chrome and before I got the prompt, the server crashed with the following.

[2023-11-04 08:12:16.669] [P:2566900] [error] [T:2566985] parse_url: curl_url_get(CURLUPART_QUERY) error: 16
Segmentation fault (core dumped)

Below is what I typed in the browser.

http://localhost:9000/irods-http-api/0.1.0/authenticate
@korydraughn korydraughn added the bug Something isn't working label Nov 4, 2023
@korydraughn korydraughn added this to the 0.1.0 milestone Nov 4, 2023
@korydraughn korydraughn self-assigned this Nov 4, 2023
@korydraughn
Copy link
Contributor Author

korydraughn commented Nov 4, 2023

This was caused by my config not having appropriate OIDC settings defined. The implementation currently relies on a GET request as part of the OIDC flow.

If we want to allow users to authenticate using Basic auth in a browser, we'll need to adjust how the HTTP API handles OIDC. Perhaps an op param can be introduced or a separate endpoint.

@korydraughn korydraughn modified the milestones: 0.1.0, 0.2.0 Nov 4, 2023
@trel
Copy link
Member

trel commented Nov 4, 2023

So we definitely need to put bumpers on that use case (missing config items).

A separate endpoint might be easier / separate the concerns.

korydraughn added a commit to korydraughn/irods_client_http_api that referenced this issue Jan 10, 2024
This commit makes it so that the server only runs OIDC authentication
code paths when the OIDC stanza is present.

Before this commit, the server would crash when the following was true:
- The configuration file did not contain an OIDC stanza.
- A user sent a GET request to the /authenticate endpoint.

The server crashed because the GET logic of the endpoint handled a
special OIDC case.
alanking pushed a commit that referenced this issue Jan 10, 2024
This commit makes it so that the server only runs OIDC authentication
code paths when the OIDC stanza is present.

Before this commit, the server would crash when the following was true:
- The configuration file did not contain an OIDC stanza.
- A user sent a GET request to the /authenticate endpoint.

The server crashed because the GET logic of the endpoint handled a
special OIDC case.
@korydraughn
Copy link
Contributor Author

Supporting authentication using the following syntax is not a requirement of the HTTP API.

http://<username>:<password>@localhost:9000/irods-http-api/0.1.0/authenticate

For that reason, this issue has been resolved since attempting to do what's described in this issue no longer results in the server crashing.

@korydraughn
Copy link
Contributor Author

Need to add tests for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants