8xFF / atm0s-media-server

Decentralized, Global-Scale Media Server written in Rust (WebRTC/Whip/Whep/Rtmp/Sip)
https://8xff.github.io/media-docs/
MIT License
235 stars 17 forks source link

Feature: External authentication/authorization check #423

Open luongngocminh opened 3 months ago

luongngocminh commented 3 months ago

Pull Request

Description

To keep the server functionalities as simple and compact as possible, currently only JWT verification is used to allow users access to a room. But due to the requirements of modern video conferencing applications, further authentication functionalities are needed (Room participant controls like kicking, banning, ...). This PR is designed to keep the authen/author out of scope of the server core functions. Developer can now extends the authentication functions by providing a sort of "guard" API HTTP base endpoint. These "guard" are placed in-front of the authenticated SDK APIs:

Changes

Related Issue

378

Checklist

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 0% with 74 lines in your changes missing coverage. Please review.

Project coverage is 40.56%. Comparing base (260ff03) to head (5ad190e).

Files Patch % Lines
bin/src/http/api_media.rs 0.00% 54 Missing :warning:
bin/src/http.rs 0.00% 18 Missing :warning:
bin/src/server/gateway.rs 0.00% 1 Missing :warning:
bin/src/server/media.rs 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #423 +/- ## ========================================== - Coverage 40.72% 40.56% -0.17% ========================================== Files 154 154 Lines 16644 16710 +66 ========================================== Hits 6778 6778 - Misses 9866 9932 +66 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

jibon57 commented 3 months ago

Nice Job 👍. One question regarding url, can't we use domain with path? Example: http://example.com/auth/webrtc-verify or similar?

giangndm commented 3 months ago

@luongngocminh I think there might be some confusion with hard-coded URL rules. Perhaps we should use a direct endpoint and include the information in the body. As we discussed with @jibon57, it's more flexible to embed the check endpoint inside the token.

luongngocminh commented 3 months ago

@jibon57 @giangndm my idea is to have a way to separate the checking logic for each concerns (in case we want to fine grain the permissions: create, patch, delete, etc. ) without branching in the target API logic. If I use a single webrtc verify API with body as the parameter, the handler logic of that API will need to be branch (using if or switch case) for each of the case. Using this method will allow more fine grain control over which action will be allow, in the case the user doesn't care about the details, they can just create a catch all route like /webrtc/* handler and handle it there.

jibon57 commented 3 months ago

@luongngocminh setting up a fixed url is totally fine. I'm just concerned the url pattern. Do we have to use only domain name without any path? Or path is allowed with domain? Because in your example you used a domain name only.

luongngocminh commented 3 months ago

@luongngocminh setting up a fixed url is totally fine. I'm just concerned the url pattern. Do we have to use only domain name without any path? Or path is allowed with domain? Because in your example you used a domain name only.

You can use any path as base, it's fine, the server will send a GET request to <your base uri>/webrtc/connect, for example http://example.com/somethingrandom/otherrandom/webrtc/connect if this is what you're asking

jibon57 commented 3 months ago

@luongngocminh thanks. Yes, that was my concern.

giangndm commented 3 months ago

@luongngocminh @jibon57 I think we shouldn't use different endpoints. Instead, we should post to the configured endpoint, allowing users to flexibly embed more information within the hook URI parameters. This also enables us to have the same mechanism when allowing custom hook URIs per token.