Skip to content

Fix Misleading title for Pages Access Control in project settings

What does this MR do?

Fixes titles of pages settings #33548 (closed)

Screenshots

old texts new texts
image Screenshot_2019-10-23_at_20.11.27

Why it works this way

I'm not sure the original intention of this feature author, but disabling access control actually disables pages altogether.

This allignes well with the rest of the controls on this page. If you disable something, e.g. snippets), it's disabled completely for everyone.

The only thing which confuses people is the text helper which suggests that you enable/disable access control, not the whole pages website.

So this MR fixes the texts.

But the way it's implemented is actually very interesting:

  1. user disable access control, i.e. sets pages_access_level to DISABLED
  2. we redeploy pages website config and enable access_control: true in it
  3. pages daemon goes through the usual authorization process
  4. when pages daemon finally tries to check if user has access to a particular pages site it call api which checks the policy https://gitlab.com/gitlab-org/gitlab/blob/master/app/policies/project_policy.rb#L333

I thought about maybe triggering cleaning pages data when a user disables pages, but decided against it, because then enabling pages would require user to run a pipeline again.

The authorization workflow doesn't add overhead, because pages daemon will go through the same authorization process if the pages website is absent in order to prevent user of knowing which sites exist and which don't.

Does this MR meet the acceptance criteria?

Conformity

Performance and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Vladimir Shushlin

Merge request reports