Autodetect and make URLs in the chat hyperlinks

[yarikoptic]

I do expect audience to provide pointers and links to other chat rooms and external websites. ATM URLs in the chat are not presented as hyperlinks, requiring dances to follow them. I think it would be great UX feature and trivial to implement to convert them (upon rendering) into <a> elements. Smart code should probably discriminate between external URLs and URLs to the event website and open external in a new window with needed security provisions. Or just open always in external since we should encourage people to stay in the chat.

Somewhat relates to discussion in https://github.com/sparkletown/sparkle/pull/1387 for the internal urls. But otherwise I think it should be safe and very useful UX feature very easy to implement.

edit:

• might interact/interfer with Emoji'fication (see #1394 )
• possible solutions are in https://stackoverflow.com/questions/33235890/react-replace-links-in-a-text . Haven't spotted any react- library mentioned there already being used but didn't look too hard

[0xdevalias]

I think it would be great UX feature and trivial to implement to convert them (upon rendering) into <a> elements. Smart code should probably discriminate between external URLs and URLs to the event website and open external in a new window with needed security provisions. Or just open always in external since we should encourage people to stay in the chat.

My initial feeling on this would be that we should probably always open them in a new window/tab.

---

Somewhat relates to discussion in #1387 for the internal urls.

The 'somewhat related' part here is probably the discussion about how having multiple Sparkle tabs open at once can interfere with the user presence 'counting' feature (see https://github.com/sparkletown/sparkle/pull/1387#pullrequestreview-659528604, https://github.com/sparkletown/sparkle/pull/1387#issuecomment-841247086, etc)

---

But otherwise I think it should be safe and very useful UX feature very easy to implement.

We need to be careful about how this gets implemented, as there are many ways that links/URLs can be turned into a security vulnerability (eg. linking to javascript:alert('do something malicious here'))

---

possible solutions are in stackoverflow.com/questions/33235890/react-replace-links-in-a-text . Haven't spotted any react- library mentioned there already being used but didn't look too hard

We actually already have this functionality within the AnnouncementMessage component (used for the banners displayed at the top screen), using getLinkFromText from src/utils/getLinkFromText.tsx.

The 'risk' of that implementation is limited to venue owners/admins at the moment, so there is less scope for a malicious user to be able to take advantage of it. I would want to assess the implementation with more scrutiny before exposing that functionality to any guest that can use the chat feature.

[yarikoptic]

But otherwise I think it should be safe and very useful UX feature very easy to implement.

We need to be careful about how this gets implemented, as there are many ways that links/URLs can be turned into a security vulnerability (eg. linking to javascript:alert('do something malicious here'))

doesn't

export const externalUrlAdditionalProps = {
  target: "_blank",
  rel: "noopener noreferrer",
};

sufficiently solve it? (I am just educating myself here ;-) )

[0xdevalias]

doesn’t externalUrlAdditionalProps sufficiently solve it? (I am just educating myself here ;-) )

That will open it in a new window/tab, and stop the ‘opener’ handle back to the calling window; so that would solve one potential class of things. Another issue (that I mentioned above) is if a link is made to a javascript:// url path (or one of the alternative paths like that that can be abused in similar ways), which can lead to XSS (cross site scripting), etc.

Basically, security is challenging, as all an attacker needs is one little hole to find their way in; and it pays to be mindful and think these things through up front. (Outside of being a software engineer, my ‘other hat’ is as an ethical hacker / penetration tester / security researcher)

[yarikoptic]

What if we restrict to https? URLs?

[0xdevalias]

What if we restrict to https? URLs?

@yarikoptic That would certainly remove that class of url based attacks and probably get a lot closer to being ‘happy’.

I’m not sure that the announcement banner should ever have a need to link to anything other than http/https urls either. So assuming these are the only places that use the function currently, we could probably tighten up the reflex being used there.

I’d still need to dedicate some brain-space to actually think deeper to see if there are other avenues in the code; which I can’t do this second as I have some other bits of code I’m reviewing/triaging/needing to get merged as well.

[mobilevinay]

migrated to zenhub, check!