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

Add auth fallback endpoint #405

Merged
merged 34 commits into from Aug 14, 2019

Conversation

@Trion129
Copy link
Contributor

commented Mar 2, 2018

Implementing endpoint for authentication fallback when client doesn't know how to handle a login type. Solving issue #398

@anoadragon453

This comment has been minimized.

Copy link
Member

commented Mar 3, 2018

Looks good so far.

@Trion129

This comment has been minimized.

Copy link
Contributor Author

commented Mar 4, 2018

JSONResponse isn't nil-able so have to look into a workaround for that

@anoadragon453

This comment has been minimized.

Copy link
Member

commented Mar 5, 2018

@Trion129 You could return a reference to JSONResponse, which would be nil-able.

@anoadragon453

This comment has been minimized.

Copy link
Member

commented Mar 14, 2018

Just an update: currently this successfully supports and displays a recaptcha object and verifies completion with the recaptcha server. Actually marking m.login.recaptcha as complete is still to be done, though may be easier accomplished once #413 has been merged.

@Trion129

This comment has been minimized.

Copy link
Contributor Author

commented Mar 15, 2018

@anoadragon453 No worries :) I am learning to write tests for routes till then

@anoadragon453

This comment has been minimized.

Copy link
Member

commented Jul 5, 2019

Hi @Trion129. Are you still interested in working on this? And if so, could you rebase your PR upon latest master to fix the conflicts please?

Thanks!

@Trion129

This comment has been minimized.

Copy link
Contributor Author

commented Jul 6, 2019

Hi @anoadragon453 , it's been so long since I saw this code. I was waiting for that other change to merge then I forgot about the PR. Sorry for that. I'll rebase the change. I will need some catching up with latest code.

@anoadragon453

This comment has been minimized.

Copy link
Member

commented Jul 6, 2019

No worries, I'll help you get caught up in #dendrite-dev:matrix.org if you need it :)

@anoadragon453

This comment has been minimized.

Copy link
Member

commented Jul 15, 2019

Getting there. In testing m.login.recaptcha doesn't seem to be added to the list of completed flows, thus inhibiting login.

@anoadragon453
Copy link
Member

left a comment

After this it should be fully functional!

Show resolved Hide resolved clientapi/routing/auth_fallback.go
@anoadragon453

This comment has been minimized.

Copy link
Member

commented Jul 19, 2019

I ended up fixing the functionality of it :)

@anoadragon453
Copy link
Member

left a comment

Great! Just some small stuff now.

Show resolved Hide resolved clientapi/routing/auth_fallback.go Outdated
Show resolved Hide resolved clientapi/routing/auth_fallback.go Outdated
Show resolved Hide resolved clientapi/routing/auth_fallback.go Outdated
Show resolved Hide resolved clientapi/routing/auth_fallback.go Outdated
Show resolved Hide resolved clientapi/routing/auth_fallback.go Outdated
Show resolved Hide resolved common/httpapi.go Outdated
Show resolved Hide resolved clientapi/routing/routing.go Outdated
Show resolved Hide resolved clientapi/routing/auth_fallback.go Outdated
Show resolved Hide resolved clientapi/routing/auth_fallback.go Outdated
Show resolved Hide resolved clientapi/routing/auth_fallback.go Outdated
@anoadragon453

This comment has been minimized.

Copy link
Member

commented Aug 9, 2019

Hmm, I expected the Register with a recaptcha test to pass with this PR.

@Cnly

This comment has been minimized.

Copy link
Collaborator

commented Aug 9, 2019

@anoadragon453 The test is failing probably because recaptcha is not enabled in the config: https://github.com/matrix-org/sytest/blob/b8d5c0a0401816a1182986af5fde1f8a3ecd3442/lib/SyTest/Homeserver/Dendrite.pm#L116 says it's deliberately disabled until matrix-org/sytest#592 is resolved.

Show resolved Hide resolved clientapi/routing/auth_fallback.go Outdated
@anoadragon453

This comment has been minimized.

Copy link
Member

commented Aug 9, 2019

@Cnly heh, I completely forgot about that. Yes, I suppose you're right.

Remove link to css file
Co-Authored-By: Alex Chen <Cnly@users.noreply.github.com>
@Cnly

This comment has been minimized.

Copy link
Collaborator

commented Aug 14, 2019

@Trion129 You didn't make the build fail. It's that SyTest updated the name of a test and we've got to update it too in the testfile on master. I'll open a PR for that and once it's merged, a merge from master onto your branch should fix the test.

@Cnly

Cnly approved these changes Aug 14, 2019

Copy link
Collaborator

left a comment

lgtm

Show resolved Hide resolved clientapi/routing/auth_fallback.go Outdated
Show resolved Hide resolved clientapi/routing/auth_fallback.go Outdated
Update comment for route
Co-Authored-By: Alex Chen <Cnly@users.noreply.github.com>
@Cnly

This comment has been minimized.

Copy link
Collaborator

commented Aug 14, 2019

@Trion129 You may now merge master to fix the build.

@Cnly

Cnly approved these changes Aug 14, 2019

Copy link
Collaborator

left a comment

Some nitpicking...

Show resolved Hide resolved clientapi/routing/auth_fallback.go Outdated
Show resolved Hide resolved clientapi/routing/auth_fallback.go Outdated
Show resolved Hide resolved clientapi/routing/auth_fallback.go Outdated
Show resolved Hide resolved clientapi/routing/auth_fallback.go Outdated
Rename writeErrorMessage to writeHTTPMessage
Co-Authored-By: Alex Chen <Cnly@users.noreply.github.com>
comment change
Co-Authored-By: Alex Chen <Cnly@users.noreply.github.com>

@Cnly Cnly merged commit d21a2fb into matrix-org:master Aug 14, 2019

8 checks passed

buildkite/dendrite Build #248 passed (3 minutes, 25 seconds)
Details
buildkite/dendrite/build-slash-go-1-dot-11 Passed (59 seconds)
Details
buildkite/dendrite/build-slash-go-1-dot-12 Passed (51 seconds)
Details
buildkite/dendrite/lint-slash-go-1-dot-12 Passed (1 minute, 51 seconds)
Details
buildkite/dendrite/pipeline Passed (8 seconds)
Details
buildkite/dendrite/unit-tests-slash-go-1-dot-11 Passed (57 seconds)
Details
buildkite/dendrite/unit-tests-slash-go-1-dot-12 Passed (1 minute, 11 seconds)
Details
ci/circleci: dendrite Your tests passed on CircleCI!
Details

@Cnly Cnly referenced this pull request Aug 14, 2019

Closed

Implement fallback web registration #398

2 of 2 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.