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

Security issues in requests calls #27

Open
fmigneault opened this issue Oct 26, 2023 · 3 comments
Open

Security issues in requests calls #27

fmigneault opened this issue Oct 26, 2023 · 3 comments
Assignees
Labels

Comments

@fmigneault
Copy link
Collaborator

All requests calls should remove enforced verify=False.
This is a debuging workaround that should not be enabled in deployed instances with valid SSL certificates.

The requests also assume open access. Realistically, most STAC API will not let user openly push new collections/items. The auth parameter must be supported to pass down an authentication/authorization method, such as https://github.com/Ouranosinc/requests-magpie

For convenience, CLI flags or utilities to pass extra arguments to requests calls could be added, but the should not enforce defaults that disable security features.

@fmigneault
Copy link
Collaborator Author

fmigneault commented Feb 25, 2025

AuthHandler code (https://github.com/crim-ca/weaver/blob/760c115c9f397499ecd2b59892af7937b9ac1cf2/weaver/cli.py#L207) and derived classes (including MagpieAuth) could be reused.

The CLI options to load/define them are here: https://github.com/crim-ca/weaver/blob/760c115c9f397499ecd2b59892af7937b9ac1cf2/weaver/cli.py#L2409-L2539

The obtained classes can then be passed to the auth parameter of the requests methods.

@mishaschwartz
Copy link
Contributor

mishaschwartz commented Feb 25, 2025

This was resolved in version 0.3.0 when the collection_processor.py script was deprecated.
The only remaining place where this is hardcoded is the stac_collection_exists function:

def stac_collection_exists(stac_host: str, collection_id: str, session: Optional[Session] = None) -> bool:
"""
Get a STAC collection
Returns the collection JSON.
"""
session = session or requests
r = session.get(os.path.join(stac_host, "collections", collection_id), verify=False)
return r.status_code == 200

which is unused and can be removed.

I recommend deleting the .deprecated directory to avoid confusion and remove unused functions like stac_collection_exists.

Authentication is handled here:

def add_request_options(parser: argparse.ArgumentParser) -> None:

@fmigneault
Copy link
Collaborator Author

Thanks @mishaschwartz
For the SSL, this is ok (already assumed fixed, I edited the existing issue just for the auth param portion).
However, I had forgotten the --auth-...parameters were already applied.
I think there is still a possible use case issue remaining about the pipeline.

The expected procedure would be at some point for the stac-populator to run on an interval on the birdhouse instance using Magpie cookies, to automatically update new files that are detected on disk (via watchdog or whatever). However, the cookies expire after some time, so they cannot be hardcoded. This forces us (or anyone using stac-populator) to write custom scripts that do the MagpieAuth or a curl requests to generate the cookiejar and forward it to stac-populator.

Since we already provide https://github.com/Ouranosinc/requests-magpie, it would be helpful to support custom classes definitions to be specified, just to make usage simplified by users that don't have the full background knowledge of the auth aspects of the platform.

I also agree about removing the .deprecated code clutter.

@Nazim-crim
See #27 (comment)
You can use the auth+cookie parameters temporarily, but you will need to do the Magpie requests by yourself in the meantime to forward the cookie to stac-populator in order to POST the items.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants