actions-hub / gcloud

GitHub Action for interacting with Google Cloud Platform (GCP)
https://github.com/marketplace/actions/google-cloud-platform-gcp-cli-gcloud
MIT License
229 stars 28 forks source link

Work without encoding the secret with base64 #13

Closed kaxil closed 4 years ago

kaxil commented 4 years ago

This PR maintains backwards-compatibility and removes the need of having a base64 encoded Secret

kaxil commented 4 years ago

cc @exelban @tibotiber

exelban commented 4 years ago

Hi. Thanks for this PR.

Maybe there is an option to do this without an additional key? Detect in some way check if passed credentials are base64 or not?

kaxil commented 4 years ago

Hi. Thanks for this PR.

Maybe there is an option to do this without an additional key? Detect in some way check if passed credentials are base64 or not?

Sure I could do that

exelban commented 4 years ago

I trying to implement this a few times. But as I remember there is no good solution to determine if a string is base64 or not. The only one solution what I found is this regex: ^([A-Za-z0-9+/]{4})*([A-Za-z0-9+/]{3}=|[A-Za-z0-9+/]{2}==)?$. There is a link with an explanation. I hope it will help.

Or maybe you will find a better solution.

kaxil commented 4 years ago

How about:

❯ orig_str='bGludXhoaW50Y29'
❯ if [ "$orig_str" = "$(echo $orig_str | base64 --decode | base64)" ]; then
echo "Base64 Encoded"; else echo "Not Base64 Not Encoded"
fi
Not Base64 Not Encoded

i.e check base64.b64encode(base64.b64decode(s)) == s

(If by decoding and encoding it again we get the same result it is a Base64 string or else it is not)

exelban commented 4 years ago

It looks like it will no work. Because you just decode -> encode the string. The problem is that base64 is a string. So you will just decode a base64 again, and encode it to the original one.

At least, these cases show that this way, will not work:

echo dGVzdA== | base64 --decode | base64
echo test | base64 --decode | base64

Both will return the $orig_str.

kaxil commented 4 years ago

It looks like it will no work. Because you just decode -> encode the string. The problem is that base64 is a string. So you will just decode a base64 again, and encode it to the original one.

At least, these cases show that this way, will not work:

echo dGVzdA== | base64 --decode | base64
echo test | base64 --decode | base64

Both will return the $orig_str.

That is because both of them are valid base64 strings though (based on https://stackoverflow.com/a/45928164/5691525)

exelban commented 4 years ago

Hmm, so the trick there is credentials are JSON object, and base64 will throw an error. And valid base64 string will just decode->encode?

exelban commented 4 years ago

So the logic here is not to check if provided credentials are invalid base64, but to check if it's a valid base64?

kaxil commented 4 years ago

yeah:

❯ export orig_str="sasdasdadada=fsfs"
❯ if [ "$orig_str" = "$(echo $orig_str | base64 --decode | base64)" ]; then
echo "Base64 Encoded"; else echo "Not Base64 Encoded"
fi
Invalid character in input stream.
Not Base64 Encoded
exelban commented 4 years ago

Thanks. I will make a few tests tonight, and merge this. Just a small ask, could you add some log to if/else? Even this one: echo "Base64 Encoded"; else echo "Not Base64 Encoded".

It will help if someone will have a problem with credentials.

kaxil commented 4 years ago

Thanks. I will make a few tests tonight, and merge this. Just a small ask, could you add some log to if/else? Even this one: echo "Base64 Encoded"; else echo "Not Base64 Encoded".

It will help if someone will have a problem with credentials.

Done (in https://github.com/actions-hub/gcloud/pull/13/commits/d20e794f89d36458f759b981fdddad4e91f23328)

exelban commented 4 years ago

It does not work(

kaxil commented 4 years ago

It does not work(

Does the test contain Base64 encoded secret or without encoding?

exelban commented 4 years ago

For now, it contains only base64 encoded

exelban commented 4 years ago

And it says that base64 encoded credentials are not encoded

kaxil commented 4 years ago

How about we simplify it and go back to the first approach i.e. instead of guessing or validating let the user tell us whrther it is encoded or not via environment variable (defaulting it to base64 encoded) ?

exelban commented 4 years ago

I just cannot understand the reason to put a json file to secret but not a base64 encoded. For me it's some standard when I need to put some structure as a string, I just encode it with base64.

If there is a way to support both methods, base64 and json in the string. I will accept it, but without any additional flags. It will make usage more complicated. If you want to put a file to secret set this key, otherwise keep it false.

kaxil commented 4 years ago

I just cannot understand the reason to put a json file to secret but not a base64 encoded. For me it's some standard when I need to put some structure as a string, I just encode it with base64.

If there is a way to support both methods, base64 and json in the string. I will accept it, but without any additional flags. It will make usage more complicated. If you want to put a file to secret set this key, otherwise keep it false.

Sure let me try something out and get back to this PR

exelban commented 4 years ago

Thanks.

Do you maybe have act installed? With this tool, you can run action locally.