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

Does no longer redirect after registration #839

Closed
NoelDeMartin opened this issue Jul 23, 2021 · 8 comments
Closed

Does no longer redirect after registration #839

NoelDeMartin opened this issue Jul 23, 2021 · 8 comments

Comments

@NoelDeMartin
Copy link
Contributor

I was using v0.9.0 for testing my application on CI, but after updating to v1.0.0-beta.0 it seems like the user is not redirected to my app after a successful registration anymore.

Here's a video of what my test is doing. It stays stuck on /idp/register without doing any redirects. Let me know if you need any more info, but I didn't see anything on the debug logs.

css-register.mp4

If the user already has an account, it does work and they are redirected to my app after logging in:

css-login.mp4

I was previously using an existing webId to register, but I've tried it and it has the same problem.

Also, I'm not sure if this is a bug or working as intended, but I noticed that new PODs don't have pim:storage or solid:privateTypeIndex in the profile. I was assuming those were always defined, but if this is not a bug I won't assume that in my apps anymore.

@joachimvh
Copy link
Member

This is working as intended currently. Although there is definitely something to be said for having this redirect.

The main reason it doesn't redirect anymore is because the registration has been pulled out of the OIDC flow, which means you can now register by going to /idp/register directly without using the authn client. But the disadvantage is that there is no redirect anymore since the registration component would have to know if it is in an OIDC flow or not, and only redirect if it is.

There are ways to add that, but it would require some changes in the IDP part probably.

The root container is still a pim:Storage, but this is not stored in the profile but in the metadata of the root container. This is the template file that gets used for the root container metadata: https://github.com/solid/community-server/blob/main/templates/pod/.meta . This information gets exposed through a link type header on the root container as defined in the spec: https://solid.github.io/specification/protocol#storage

@joachimvh
Copy link
Member

I should probably also add that the pod templates are not set in stone and could certainly be updated if needed. It's also possible to configure the server to use your own set of templates if you want, but I get that from an app point of view you want to know what to possibly expect in a profile.

@RubenVerborgh
Copy link
Member

But the disadvantage is that there is no redirect anymore since the registration component would have to know if it is in an OIDC flow or not

Can't we just pass a redirect query string parameter to the registration flow?

@joachimvh
Copy link
Member

Can't we just pass a redirect query string parameter to the registration flow?

That could be a solution for now. Although it's shifting some extra logic to the registration handler we probably want to get rid of again at some point.

@NoelDeMartin
Copy link
Contributor Author

NoelDeMartin commented Jul 23, 2021

The main reason it doesn't redirect anymore is because the registration has been pulled out of the OIDC flow, which means you can now register by going to /idp/register directly without using the authn client. But the disadvantage is that there is no redirect anymore since the registration component would have to know if it is in an OIDC flow or not, and only redirect if it is.

Ok if that's expected feel free to close the issue then, I can do the redirect myself in the test suite. If you think it's a worthy improvement, leave it open :). I don't know how many real users will end up seeing this, but if they do I think it'd be a better approach because if they are stuck on the "registration successful" page, they may not know how to go back to the app. I tried clicking the links on screen and I got turtle documents, which may be even more confusing for users.

The root container is still a pim:Storage, but this is not stored in the profile but in the metadata of the root container. This is the template file that gets used for the root container metadata: https://github.com/solid/community-server/blob/main/templates/pod/.meta . This information gets exposed through a link type header on the root container as defined in the spec: https://solid.github.io/specification/protocol#storage

I see, thanks for pointing that out. It's a bummer that I'll have to do multiple requests now, as it mentions on the spec "Clients can determine the storage of a resource by moving up the URI path hierarchy until the response includes a Link header with rel="type" targeting http://www.w3.org/ns/pim/space#Storage."

But if that's how it's specified, that's it.

And do you know something about the type index? It seems like it's not on the spec and I read about it on the proposals folder, so I guess it's non-standard. But it's very useful for interoperability. Maybe I could create it if it doesn't exist, I don't think that should be a problem for users who don't have it.

@RubenVerborgh
Copy link
Member

Tagging @acoburn for an opinion on registration flow.

And do you know something about the type index? It seems like it's not on the spec and I read about it on the proposals folder, so I guess it's non-standard. But it's very useful for interoperability. Maybe I could create it if it doesn't exist, I don't think that should be a problem for users who don't have it.

That behavior is not standardized indeed; feel free to create, I would say.

@NoelDeMartin
Copy link
Contributor Author

Hey, I just tried v1.0.0-beta.1 and maybe this issue can be closed, because there is a new link to log in, and it redirects properly to the app after that:

css-register+login.mp4

I'm not closing it in case you still want to talk about redirecting right after registration, but I would say it's fixed :).

@RubenVerborgh
Copy link
Member

Excellent, thanks a ton, @NoelDeMartin.

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

No branches or pull requests

3 participants