Skip to content
This repository has been archived by the owner on Jul 12, 2023. It is now read-only.

Add self report #1930

Merged
merged 9 commits into from
Mar 22, 2021
Merged

Add self report #1930

merged 9 commits into from
Mar 22, 2021

Conversation

mikehelmick
Copy link
Contributor

Towards #1928

Proposed Changes

  • Adds API for UserReport to request a verification code
  • requests are de-duplicated for phone number over 90d

New checkbox to enable user report

image

New SMS template for "User Report"

image

Release Note

Add support for self-report initiation and certification. This feature is disabled by default, enable by setting `ENABLE_USER_REPORT` to `true`

@mikehelmick mikehelmick requested a review from a team as a code owner March 20, 2021 00:29
@google-cla google-cla bot added the cla: yes Auto: added by CLA bot when all committers have signed a CLA. label Mar 20, 2021
if errors.Is(err, database.ErrAlreadyReported) {
return &IssueResult{
obsResult: enobs.ResultError("DUPLICATE_USER_REPORT"),
HTTPCode: http.StatusTooManyRequests,
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like StatusTooManyRequests implies rate-limiting? Would conflict be more appropriate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

other ideas, just 400? 403?

Copy link
Member

Choose a reason for hiding this comment

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

409 would be most appropriate. I think we need to document the response and explain what clients should do instead.

internal/clients/api_server.go Show resolved Hide resolved
pkg/api/api.go Show resolved Hide resolved
pkg/api/api.go Show resolved Hide resolved
pkg/config/admin_server_config.go Show resolved Hide resolved
pkg/controller/cleanup/handle_cleanup.go Outdated Show resolved Hide resolved
pkg/database/user_report.go Show resolved Hide resolved
pkg/database/user_report.go Outdated Show resolved Hide resolved
pkg/database/user_report.go Show resolved Hide resolved
pkg/database/vercode.go Outdated Show resolved Hide resolved
# Create secret for the database HMAC for phone numbers
resource "random_id" "db-phone-number-hmac" {
count = var.db_phone_number_hmac_count
byte_length = 128
Copy link
Member

Choose a reason for hiding this comment

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

Some of the comments say 256?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the HMAC secret length, not the nonce length

@mikehelmick
Copy link
Contributor Author

updated + added api docs.

pkg/controller/realmadmin/settings_modify.go Outdated Show resolved Hide resolved
pkg/controller/verifyapi/verify.go Outdated Show resolved Hide resolved
pkg/database/database.go Show resolved Hide resolved
pkg/database/realm.go Outdated Show resolved Hide resolved
pkg/database/realm.go Show resolved Hide resolved
pkg/database/token_test.go Outdated Show resolved Hide resolved
pkg/database/user_report.go Show resolved Hide resolved
pkg/database/user_report.go Show resolved Hide resolved
pkg/database/user_report.go Outdated Show resolved Hide resolved
pkg/database/vercode_test.go Outdated Show resolved Hide resolved
@sethvargo
Copy link
Member

Test failures seem real?

sethvargo
sethvargo previously approved these changes Mar 22, 2021
@mikehelmick mikehelmick enabled auto-merge (squash) March 22, 2021 19:48
@mikehelmick mikehelmick merged commit d7ec6c6 into google:main Mar 22, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes Auto: added by CLA bot when all committers have signed a CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants