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

server: disable security on /api/v2/ when running insecure #86417

Merged
merged 1 commit into from
Aug 19, 2022

Conversation

dhartunian
Copy link
Collaborator

Previously, the session validation log in /api/v2/ would still run even when
the cluster was running in "insecure" mode. This made it cumbersome to test new
features while developing, and also caused new challenges when developing DB
Console features that use /api/v2/ endpoints.

Now, when the cluster has the insecure flag set to true, web session check
failures won't cause failures on endpoints and the session username will be set
to "root" automatically in the context.

Release note (security update): HTTP API endpoints under the /api/v2/ prefix,
will allow requests through when the cluster is running in "insecure" mode.
When the cluster is running in "insecure" mode requests to these endpoints will
have the username set to "root".

Release justification: low-risk high-benefit change to existing functionality.

@dhartunian dhartunian requested review from knz, xinhaoz and a team August 18, 2022 20:09
@dhartunian dhartunian requested review from a team as code owners August 18, 2022 20:09
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@knz knz left a comment

Choose a reason for hiding this comment

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

thanks

}

return username, sessionCookie, nil
return username, sessionCookie, 200, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

http.StatusOK?

Previously, the session validation log in `/api/v2/` would still run even when
the cluster was running in "insecure" mode. This made it cumbersome to test new
features while developing, and also caused new challenges when developing DB
Console features that use `/api/v2/` endpoints.

Now, when the cluster has the insecure flag set to true, web session check
failures won't cause failures on endpoints and the session username will be set
to "root" automatically in the context.

Release note (security update): HTTP API endpoints under the `/api/v2/` prefix,
will allow requests through when the cluster is running in "insecure" mode.
When the cluster is running in "insecure" mode requests to these endpoints will
have the username set to "root".

Release justification: low-risk high-benefit change to existing functionality.
@dhartunian dhartunian force-pushed the api-v2-insecure-cookie-support branch from c9e7d1d to 109aac2 Compare August 18, 2022 21:16
Copy link
Collaborator Author

@dhartunian dhartunian left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @knz and @xinhaoz)


pkg/server/api_v2_auth.go line 351 at r1 (raw file):

Previously, knz (kena) wrote…

http.StatusOK?

Done.

@dhartunian
Copy link
Collaborator Author

TFTR

bors r=knz

@craig
Copy link
Contributor

craig bot commented Aug 19, 2022

Build succeeded:

@craig craig bot merged commit c4d4a0b into cockroachdb:master Aug 19, 2022
@blathers-crl
Copy link

blathers-crl bot commented Aug 19, 2022

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.


error creating merge commit from 109aac2 to blathers/backport-release-22.1-86417: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 22.1.x failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

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