feat: avoid using pins for interpretation rooms #24
Conversation
marcos-cereijo-pexip
force-pushed
the
feat/avoid-using-pins-for-the-interpretation-rooms
branch
5 times, most recently
from
e8cd769 to
0987ee7
Compare
There was a problem hiding this comment.
Pull request overview
This PR implements a new authentication mechanism for interpretation rooms that uses hash-based PINs derived from call tags instead of static PINs. The main change introduces a pexHash function to generate secure, deterministic PINs based on the user's call tag and role, improving security by avoiding hardcoded credentials.
Key Changes
- Added
pexHashutility function to generate SHA-256-based hash values for dynamic PIN generation - Refactored call setup logic to extract call type determination into a separate
getCallTypefunction - Updated documentation with detailed explanations of the new call tag-based authentication flow
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/utils.ts | Adds new pexHash function to generate SHA-256 hashes for dynamic PIN creation |
| src/InterpretationContext/InterpretationContext.tsx | Refactors connection logic to use call tag-based PINs and extracts call type logic into separate function |
| docs/call_tag.puml | Adds sequence diagram documenting the call tag authentication flow |
| README.md | Updates documentation with comprehensive explanation of the new PIN generation mechanism and testing procedures |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
marcos-cereijo-pexip
force-pushed
the
feat/avoid-using-pins-for-the-interpretation-rooms
branch
7 times, most recently
from
cc6e2b8 to
eb29647
Compare
README.md
Outdated
| {% elif (call_info.local_alias | pex_regex_replace('^(\d{6})$', '') == '') %} | ||
| {# Interpretation rooms for 6-digit VMRs #} | ||
|
|
||
| {% set callTag = ((call_info.local_alias | pex_regex_replace('^(\d{2})(\d{4})$', '\\1') ) + (call_info.remote_display_name | pex_regex_replace('^(.*)\ -\ (Interpreter|Listener)$', '\\1') )) | pex_hash | pex_tail(20) %} |
There was a problem hiding this comment.
The callTag is propagated to every participant in the VMR. Could this be a security issue ?
Would the private_custom_properties field be better ?
There was a problem hiding this comment.
Could it make sense to have a threat modelling session with @matthooper1 ?
There was a problem hiding this comment.
Is it already in v39? Should we only ping Matt if we choose the callTag or should we ping in any case?
There was a problem hiding this comment.
I would at least ask him, it is quite sensible ? Maybe I get it wrong, but to propagate a kind of credential to each client should be discussed.
marcos-cereijo-pexip
force-pushed
the
feat/avoid-using-pins-for-the-interpretation-rooms
branch
from
2411d3b to
de60bd5
Compare
Closes #23