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

Use mask_secrets properly #2754

Merged
merged 10 commits into from
Jun 17, 2016
Merged

Use mask_secrets properly #2754

merged 10 commits into from
Jun 17, 2016

Conversation

manasdk
Copy link
Contributor

@manasdk manasdk commented Jun 16, 2016

  • Use the mask_secrets property from API config
  • Provide API support for admin to override masking

Note : --show-secrets is not supported for st2 apikey get. This feature is meant to enable token export.

CLI

With and without --show-secrets

(virtualenv)vagrant@st2dev:~/st2$ st2 apikey list -dy
-   created_at: '2016-06-17T00:08:05.555610Z'
    enabled: true
    id: 57633f65d9d7ed1fc39c019e
    key_hash: '********'
    metadata: {}
    uid: '********'
    user: stanley

(virtualenv)vagrant@st2dev:~/st2$ st2 apikey list -dy --show-secrets
-   created_at: '2016-06-17T00:08:05.555610Z'
    enabled: true
    id: 57633f65d9d7ed1fc39c019e
    key_hash: f157ebc6ae10f87af9205b31e9492d156fbd036b12686d97c69536b04d8c05327d80fc467ffc7de6f0f518b86e860a8d06aff289ed81a66bdcb160b8f23a1add
    metadata: {}
    uid: api_key:f157ebc6ae10f87af9205b31e9492d156fbd036b12686d97c69536b04d8c05327d80fc467ffc7de6f0f518b86e860a8d06aff289ed81a66bdcb160b8f23a1add
    user: stanley

Todo

* utility method in a base class that controllers can use to decide
  the value of mask_secrets. This utility uses query parameters to
  decide if secrets should be returned unmasked.
* apikeys controller uses utility to decide
@manasdk manasdk added RFR and removed WIP labels Jun 17, 2016
@Kami
Copy link
Member

Kami commented Jun 17, 2016

I'm personally not too opinionated about this - I think displaying a hash itself is fine since it's not a secret.

@@ -64,3 +70,25 @@ def _get_query_param_value(self, request, param_name, param_type, default_value=
value = transform_to_bool(value)

return value

def _get_mask_secrets(self, request):
Copy link
Member

Choose a reason for hiding this comment

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

👍

@manasdk
Copy link
Contributor Author

manasdk commented Jun 17, 2016

I think displaying a hash itself is fine since it's not a secret.

I was starting to question that assumption as well. Anyway, this is more or less a continuation of a previous decision so keeping with it until we conclude that key_hash is not a secret anyway.

@Kami
Copy link
Member

Kami commented Jun 17, 2016

Yeah, hash is by definition one-way so there should be no reason to hide it or treat is as a secret.

Having said that, I'm also fine with doing it, but I guess it could confuse some users and make them thing like we use symmetric encryption or something instead of hash if we mask it...

@manasdk
Copy link
Contributor Author

manasdk commented Jun 17, 2016

Having said that, I'm also fine with doing it

Keeping it as is for now.

* master:
  send bastion parameter through to paramiko ssh client
@manasdk manasdk merged commit c220652 into master Jun 17, 2016
@manasdk manasdk deleted the use_right_mask branch June 17, 2016 18:41
@cognifloyd cognifloyd removed the RFR label Aug 9, 2021
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.

None yet

3 participants