Skip to content

Mask the value of the Authorization header if debug is enabled#501

Merged
lgarber-akamai merged 4 commits intolinode:mainfrom
rosskirkpat:sanitize-debug-header
May 14, 2024
Merged

Mask the value of the Authorization header if debug is enabled#501
lgarber-akamai merged 4 commits intolinode:mainfrom
rosskirkpat:sanitize-debug-header

Conversation

@rosskirkpat
Copy link
Contributor

📝 Description

What does this PR do and why is this change necessary?

If LINODE_DEBUG is enabled, the resty debug returns the plain-text Authorization header value from the request. This PR ensures that the Authorization header value will be sanitized/masked if debug mode is enabled.

I also added a logger to the internal testutil package that is compliant with the resty.Logger interface.

✔️ How to Test

What are the steps to reproduce the issue or verify the changes?

Reproduce the issue: set LINODE_DEBUG when using linodego and observe a plain-text token in the debug output from resty ie. Authorization: Bearer <LINODE_TOKEN_PLAIN_TEXT>

Verify the changes: set LINODE_DEBUG when using linodego and observe a masked token in the debug output from resty (see below).

2024/05/07 12:58:21.997176 DEBUG RESTY 
==============================================================================
~~~ REQUEST ~~~
GET  /v4/linode/instances  HTTP/1.1
HOST   : api.linode.com
HEADERS:
	Accept: application/json
	Authorization: Bearer *******************************
	Content-Type: application/json
	User-Agent: linodego/dev https://github.com/linode/linodego
BODY   :
***** NO CONTENT *****
------------------------------------------------------------------------------
~~~ RESPONSE ~~~
STATUS       : 200
PROTO        : 
RECEIVED AT  : 2024-05-07T12:58:21.997154-04:00
TIME DURATION: 47.792µs
HEADERS      :
	Content-Type: application/json
BODY         :
{
   "id": 100,
   "region": "test-central",
   "label": "this-is-a-test-linode"
}
==============================================================================

How do I run the relevant unit/integration tests?

I added a new test TestDebugLogSanitization relating to these changes.

Signed-off-by: Ross Kirkpatrick <rkirkpat@akamai.com>
Signed-off-by: rosskirkpat <rosskirkpat@outlook.com>
@rosskirkpat rosskirkpat requested a review from a team as a code owner May 7, 2024 17:52
@rosskirkpat rosskirkpat requested review from jriddle-linode and yec-akamai and removed request for a team May 7, 2024 17:52
Signed-off-by: Ross Kirkpatrick <rosskirkpat@outlook.com>
@jriddle-linode jriddle-linode added the improvement for improvements in existing functionality in the changelog. label May 8, 2024
Copy link
Collaborator

@jriddle-linode jriddle-linode left a comment

Choose a reason for hiding this comment

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

Nice catch thank you.

Copy link
Contributor

@yec-akamai yec-akamai left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! Everything looks great aside from @lgarber-akamai 's comment 👍

Copy link
Contributor

@lgarber-akamai lgarber-akamai left a comment

Choose a reason for hiding this comment

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

I'll be out Thursday and Friday so I'll give this my preemptive approval for once my comment has been addressed. Thanks for the contribution!

@rosskirkpat
Copy link
Contributor Author

I'll be out Thursday and Friday so I'll give this my preemptive approval for once my comment has been addressed. Thanks for the contribution!

@lgarber-akamai When you have a minute, would I be able ant to get your sign-off on the latest commit? I believe this PR is ready to be merged now.

Copy link
Contributor

@lgarber-akamai lgarber-akamai left a comment

Choose a reason for hiding this comment

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

Looks perfect, thank you!

@lgarber-akamai lgarber-akamai merged commit 25fe7d3 into linode:main May 14, 2024
@rosskirkpat rosskirkpat deleted the sanitize-debug-header branch May 14, 2024 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement for improvements in existing functionality in the changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants