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

[FEATURE] add authorization header to httpstep #170

Merged
merged 35 commits into from
Feb 26, 2025

Conversation

dannymeijer
Copy link
Member

@dannymeijer dannymeijer commented Feb 26, 2025

Description

This pull request includes changes to the HttpStep class and adds comprehensive unit tests. The changes to the HttpStep class include:

  • Addition of decode_sensitive_headers method to decode SecretStr values in headers.
  • Modification of get_headers method to dump headers into JSON without SecretStr masking.
  • Addition of methods (get, post, put, delete) to handle different HTTP methods.
  • Addition of the auth_header field to handle authorization headers, replacing the previous implementation.

The unit tests cover:

  • Successful and failed HTTP requests for GET, POST, PUT, and DELETE methods.
  • Handling of authorization headers, including Bearer tokens and Digest authentication.
  • Retry logic for handling HTTP errors with configurable retry counts.
  • Backoff strategies for retrying requests with exponential backoff.

Related Issue

#162

Motivation and Context

This pull request addresses potential data leaks regarding authorization headers. The changes ensure that sensitive information is handled securely and that the HttpStep class can handle various HTTP methods. The comprehensive unit tests help prevent regressions and ensure that the class behaves as expected under different conditions.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

dannymeijer and others added 30 commits January 31, 2025 20:36
…nd improve test coverage. Restore to 'normal' python string behavior.
…safe and unsafe contexts + leave handling of __format__ to the super class in unsafe scenario
…lication support in SecretStr and SecretBytes
…e logging in nested inheritance tests + improved test
…BaseModel for consistency; update related tests
…with requests_mock in HTTP tests for clarity
@dannymeijer dannymeijer added this to the 0.10 milestone Feb 26, 2025
@dannymeijer dannymeijer self-assigned this Feb 26, 2025
@dannymeijer dannymeijer added bug Something isn't working enhancement New feature or request labels Feb 26, 2025
Copy link
Contributor

@zarembat zarembat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat 😊

Copy link
Contributor

@pariksheet pariksheet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dannymeijer dannymeijer marked this pull request as ready for review February 26, 2025 15:07
@dannymeijer dannymeijer requested a review from a team as a code owner February 26, 2025 15:07
@dannymeijer dannymeijer changed the title [WORK IN PROGRESS] 162 feature add authorization header to httpstep [FEATURE] add authorization header to httpstep Feb 26, 2025
@dannymeijer dannymeijer merged commit 744fab6 into main Feb 26, 2025
15 checks passed
@dannymeijer dannymeijer deleted the 162-feature-add-authorization_header-to-httpstep branch February 26, 2025 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[FEATURE] Add authorization_header to HttpStep to support other token types besides just bearer
3 participants