Skip to content

[v15] Remove account lockout from failed recovery attempts#37196

Merged
jentfoo merged 1 commit intobranch/v15from
jent/remove_recovery_failure_lockout-v15
Jan 25, 2024
Merged

[v15] Remove account lockout from failed recovery attempts#37196
jentfoo merged 1 commit intobranch/v15from
jent/remove_recovery_failure_lockout-v15

Conversation

@jentfoo
Copy link
Copy Markdown
Contributor

@jentfoo jentfoo commented Jan 24, 2024

v15 backport of PR #35325, conflicts were isolated to api/types/types.pb.go

changelog: Accounts are not locked after repeated recovery failures

* Remove account lockout from failed recovery attempts

This account lockout looks to be unecessary and potentially problematic.  Recovery codes and recovery through MFA are not possible to brute force.

In addition the potential to lockout an account from being able to use a recovery method could result in them being unable to unlock their account from other potential abuse cases (for example an attacker locking the account from failed password attempts).

As discussed in the RFD (#35533) this includes the removal of all the API used for this locking mechanism.

* accountrecovery: Update `WithLock` function names

* accountrecovery: Combine verifyRecoveryCode and verifyRecoveryCodeWithRecord into one function
@github-actions
Copy link
Copy Markdown
Contributor

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@github-actions github-actions Bot requested a review from xinding33 January 24, 2024 20:17
@github-actions
Copy link
Copy Markdown
Contributor

🤖 Vercel preview here: https://docs-k6xf7mkhh-goteleport.vercel.app/docs/ver/preview

@jentfoo
Copy link
Copy Markdown
Contributor Author

jentfoo commented Jan 24, 2024

/excludeflake *

1 similar comment
@zmb3
Copy link
Copy Markdown
Collaborator

zmb3 commented Jan 24, 2024

/excludeflake *

@jentfoo jentfoo added this pull request to the merge queue Jan 24, 2024
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jan 24, 2024
@jentfoo jentfoo added this pull request to the merge queue Jan 24, 2024
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jan 24, 2024
@jentfoo jentfoo added this pull request to the merge queue Jan 24, 2024
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jan 24, 2024
@zmb3
Copy link
Copy Markdown
Collaborator

zmb3 commented Jan 24, 2024

This is a little bit concerning - haven't seen this failure before:

time="2024-01-24T23:05:13Z" level=error msg="Application failed" error="signal: killed" test=TestIntegrationSSH/TestSSH
--- FAIL: TestIntegrationSSH (14.79s)
    --- FAIL: TestIntegrationSSH/TestSSH (14.79s)
        suite.go:[17](https://github.com/gravitational/teleport/actions/runs/7647288725/job/20837913467#step:7:18)3: 
            	Error Trace:	/__w/teleport/teleport/integrations/lib/testing/integration/suite.go:173
            	            				/opt/go/src/testing/testing.go:1169
            	            				/opt/go/src/testing/testing.go:1347
            	            				/opt/go/src/testing/testing.go:1589
            	Error:      	Received unexpected error:
            	            	signal: killed
            	Test:       	TestIntegrationSSH/TestSSH
FAIL
FAIL	github.com/gravitational/teleport/integrations/lib/testing/integration	[21](https://github.com/gravitational/teleport/actions/runs/7647288725/job/20837913467#step:7:22).642s
ok  	github.com/gravitational/teleport/integrations/lib/watcherjob	4.017s
FAIL

@jentfoo
Copy link
Copy Markdown
Contributor Author

jentfoo commented Jan 24, 2024

It's unclear how these changes would have impacted ssh, and it's not a failure I saw during the master PR.

So far the merge failure has been due to three different causes:

  1. Network failure in MacOS Build
  2. Go Unit test failure
  3. Integration test failure you mentioned

So I believe it's all unrelated, and just bad luck. I will try again and see if we any of the failures become persistent. But let me know if you think there may be a possible interaction I am not considering. Thank you!

@jentfoo jentfoo added this pull request to the merge queue Jan 24, 2024
Merged via the queue into branch/v15 with commit f476bc6 Jan 25, 2024
@jentfoo jentfoo deleted the jent/remove_recovery_failure_lockout-v15 branch January 25, 2024 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants