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

filter user e-mail in graph/user requests #8764

Merged

Conversation

dragonchaser
Copy link
Member

@dragonchaser dragonchaser commented Apr 2, 2024

Bugfix: Mask user email in output

We have fixed a bug where the user email was not masked in the output and the user emails could be enumerated through
the sharee search. This is the ocis side which adds an suiting config option to mask user emails in the output.

refs #8726

Signed-off-by: Christian Richter <crichter@owncloud.com>
@owncloud owncloud deleted a comment from update-docs bot Apr 2, 2024
@dragonchaser dragonchaser marked this pull request as ready for review April 2, 2024 14:44
Copy link
Contributor

@rhafer rhafer left a comment

Choose a reason for hiding this comment

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

I think the config option is misplaced. We should also fix that for ocs. (Also I somewhat disagree on the requirement to have this configurable at all. We already have far too many knobs. But I guess that is not my call)

@@ -2,5 +2,6 @@ package config

// TokenManager is the config for using the reva token manager
type TokenManager struct {
JWTSecret string `yaml:"jwt_secret" env:"OCIS_JWT_SECRET;GRAPH_JWT_SECRET" desc:"The secret to mint and validate jwt tokens." introductionVersion:"pre5.0"`
JWTSecret string `yaml:"jwt_secret" env:"OCIS_JWT_SECRET;GRAPH_JWT_SECRET" desc:"The secret to mint and validate jwt tokens." introductionVersion:"pre5.0"`
ShowUserEmailInResults bool `yaml:"mask_user_email" env:"OCIS_SHOW_USER_EMAIL_IN_RESULTS" desc:"Mask user email addresses in responses." introductionVersion:"5.1"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you put the setting in the TokenManager struct. I don't think it should go there.

Wouldn't it better fit into config.API? See config.go

The same it true for the ocs service btw.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, if it is still for debate if we need this switch or not, I am team Ralf 😃

Copy link
Member Author

Choose a reason for hiding this comment

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

I already adapted the code

Signed-off-by: Christian Richter <crichter@owncloud.com>
Copy link

sonarcloud bot commented Apr 3, 2024

@dragonchaser dragonchaser requested a review from rhafer April 3, 2024 11:45
@dragonchaser dragonchaser merged commit 9ba18a9 into owncloud:master Apr 3, 2024
4 checks passed
@dragonchaser dragonchaser deleted the filter-user-email-in-results branch April 3, 2024 12:51
ownclouders pushed a commit that referenced this pull request Apr 3, 2024
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.

4 participants