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

Fixe a use-case when scope path is not set yet #50

Merged
merged 1 commit into from
Feb 8, 2024

Conversation

emejia-mdsol
Copy link
Contributor

@emejia-mdsol emejia-mdsol commented Feb 6, 2024

This PR also fixes an issue when the private key has \n characters, for example, copying a private key from Medistrano and using the insomnia plugin to make it a one-liner.

https://github.com/mdsol/mauth-insomnia-plugin/blob/master/doc/install_guide.md#converting-private-keys

"-----BEGIN RSA PRIVATE KEY-----\nMIIEnw...F+H\n-----END RSA PRIVATE KEY-----"

Merging this PR will fix this deployment.
https://service.sumologic.com/ui/#/search/create?id=UpyuWYAE1CYLchXGppQNsgwnqeGRCPKzfr7m6jbN

@@ -20,4 +20,4 @@ def _get_private_key():
except ModuleNotFoundError:
pass

return private_key.replace(" ", "\n").replace("\nRSA\nPRIVATE\nKEY", " RSA PRIVATE KEY")
return private_key.replace("\\n", " ").replace(" ", "\n").replace("\nRSA\nPRIVATE\nKEY", " RSA PRIVATE KEY")
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, seems like the name of the file is rather deceptive if this code it affecting things other than Lambdas? @ykitamura-mdsol

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, good point. We used to use this only in Lambdas but not anymore.
Since renaming will break backward compatibility, I think it will require a major version upgrade.
Probably we should do it in a separate PR.

Copy link
Contributor

@ejinotti-mdsol ejinotti-mdsol left a comment

Choose a reason for hiding this comment

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

looks more or less ok to me. the main needed things are:

  • version bump and changelog entry (see other recent PRs).
  • some kinda test(s) that fails under the previous code and is fixed by the changes here.

@emejia-mdsol emejia-mdsol force-pushed the fix/asgi-middleware-scope branch from af2d50b to d9bd6fc Compare February 8, 2024 22:03
@emejia-mdsol
Copy link
Contributor Author

This happens within the context of a server starting up, making it difficult to add unit tests to this PR as it is not part of the dependencies of this project.
I did add the version bump and changelog entry.

@emejia-mdsol emejia-mdsol force-pushed the fix/asgi-middleware-scope branch from d9bd6fc to 2881f42 Compare February 8, 2024 22:05
@ykitamura-mdsol ykitamura-mdsol merged commit d126996 into main Feb 8, 2024
17 checks passed
@ykitamura-mdsol ykitamura-mdsol deleted the fix/asgi-middleware-scope branch February 8, 2024 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants