Easy login Support #3544
Easy login Support #3544
Conversation
|
@devinbinnie don't know if the problem is me, but for some reason when I try to open the link after the server has been added previously, it still does not navigate properly.. I think the whole thing is strange, for example
any idea why that would be ? I've search through the codebase to see if it was possibly related to the |
@enahum Seems to be working okay for me - I can open the link from a browser and the URL in the tab changes. |
|
During our convo we found that the webapp may be blocking the request to the server if the path contains login and we would need to add a special case. @devinbinnie can you expand a bit more, I don't remember the details |
|
@devinbinnie Added the changes to open the login screen properly even if the server is already added. |
src/app/navigationManager.ts
Outdated
| const parsedURL = parseURL(url)!; | ||
| const server = ServerManager.lookupServerByURL(parsedURL, true); | ||
|
|
||
| let trimmedPathname = getFormattedPathName(parsedURL.pathname); |
There was a problem hiding this comment.
I don't think this is necessary, getFormattedPathName basically just adds a / to the end of the pathname:
export const getFormattedPathName = (pn: string) => (pn.endsWith('/') ? pn : `${pn}/`);
So if we want to remove it, we should just use parsedURL.pathname
src/common/utils/url.ts
Outdated
| } | ||
|
|
||
| // Also allow for URLs where /login/sso/easy is not at root (e.g., subpath) | ||
| const match = url.match(/\/login\/sso\/easy\?t=([A-Za-z0-9]{64})/); |
There was a problem hiding this comment.
We should avoid a regex here, they are susceptible to path traversal. Instead, we should use the isUrlType function above and match on that. It's been reviewed by security so it should be safe :)
src/common/utils/url.ts
Outdated
| const parsed = parseURL(urlToTest); | ||
|
|
||
| // Check pathname and search params | ||
| if (parsed?.pathname === '/login/one_time_link') { |
There was a problem hiding this comment.
This should also use isUrlType. We should also check for subpath on this one too.
src/app/navigationManager.ts
Outdated
| } | ||
| } else if (ServerManager.hasServers()) { | ||
| ServerHub.showNewServerModal(`${parsedURL.host}${getFormattedPathName(parsedURL.pathname)}${parsedURL.search}`); | ||
| ServerHub.showNewServerModal(`${parsedURL.host}${trimmedPathname}${parsedURL.search}`); |
There was a problem hiding this comment.
Was getFormattedPathName not necessary here anymore? I think we needed that to ensure that we matched on the correct URL object (since it always appends it when doing toString())
There was a problem hiding this comment.
Not my code, but my reasoning around this is...
We don't use parsedURL.toString() because we want to remove the protocol (don't know why, but I see no other reason).
And we are joining host + path + searchquery. If path has a slash at the end it feels weird (www.website.com/something/?a=b instead of www.website.com/something?a=b).
No strong feelings here, so happy revert.
| e: IpcMainInvokeEvent, | ||
| url?: string, | ||
| currentId?: string, | ||
| originalUrl?: string, |
There was a problem hiding this comment.
This doesn't look like it's being used anywhere, can we remove it?
Summary
this PR introduces the EasyLogin for Guests accounts. This is part of the project of Build 2025
Ticket Link
TDB
Release Note