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

Add ice auth support #37

Merged
merged 4 commits into from Oct 27, 2022
Merged

Add ice auth support #37

merged 4 commits into from Oct 27, 2022

Conversation

johanhelsing
Copy link
Owner

@johanhelsing johanhelsing commented Jul 24, 2022

  • expose api for setting authentication
  • native implementation
  • wasm implementation
  • should credential and username be String or Option<String>?
  • document new api
  • [ ] implement From trait for new enum

Fixes #3

@johanhelsing johanhelsing added the enhancement New feature or request label Jul 24, 2022
@tedsteen
Copy link
Contributor

should credential and username be String or Option?

Had a quick look, what about something like this?

impl Default for RtcIceServerConfig {
    fn default() -> Self {
        Self {
            urls: vec![
                "stun:stun.l.google.com:19302".to_string(),
                //"stun:stun.johanhelsing.studio:3478".to_string(),
                //"turn:stun.johanhelsing.studio:3478".to_string(),
            ],
            credentials: Default::default(),
        }
    }
}
#[derive(Debug)]
pub struct RtcIcePasswordCredentials {
    username: String,
    credential: String,
}

#[derive(Debug)]
pub struct RtcIceOathCredentials {
    //Todo
}

#[derive(Debug)]
pub enum RtcIceCredentials {
    None,
    Password(RtcIcePasswordCredentials),
    Oauth(RtcIceOathCredentials),
}

impl Default for RtcIceCredentials {
    fn default() -> Self {
        RtcIceCredentials::None
    }
}

Copy link
Contributor

@tedsteen tedsteen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't use the RtcIceCredentialType yet as it's not exposed. I made a comment about it

}

#[derive(Debug)]
pub enum RtcIceCredentialType {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tedsteen
Copy link
Contributor

tedsteen commented Jul 25, 2022

Pushed my code as I had it lying around, note that target branch is turn-support, not main
#38

@tedsteen
Copy link
Contributor

WDYT about my suggestion over here? #38

@johanhelsing johanhelsing marked this pull request as ready for review October 27, 2022 07:16
@johanhelsing
Copy link
Owner Author

@tedsteen I think this should be good to go now. I haven't tested it, though, as I don't have a turn server running. Would you mind testing both implementations with yours?

@tedsteen
Copy link
Contributor

I've been using a hosted solution https://xirsys.com/ for my turn needs and I've tried the RtcIceCredentials::Password on that and they work fine.

I've also tried the IceCredentials::None and they still work :)

@johanhelsing
Copy link
Owner Author

@tedsteen Thanks!

@johanhelsing johanhelsing merged commit 9d5a1e2 into main Oct 27, 2022
@johanhelsing johanhelsing deleted the turn-support branch October 27, 2022 07:51
@PraxTube
Copy link

@tedsteen have you a working example of how to use a TURN server? I am currently trying to figure out how this works but I am a little lost and could need a reference point.

@tedsteen
Copy link
Contributor

@tedsteen have you a working example of how to use a TURN server? I am currently trying to figure out how this works but I am a little lost and could need a reference point.

Hi!
Here is an example where I use turn servers with credentials https://github.com/tedsteen/nes-bundler/blob/4df0f60cc1866547d5d553c66089bfc4f7c1263e/src/netplay/connecting_state.rs#L207

For the servers themselves I use a service https://xirsys.com
They have a really nice free tier that will be enough for most of your needs for sure if you are using it for ggrs.

@PraxTube
Copy link

Thanks! I appreciate the fast reply. Yeah xirsys looks pretty good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TURN support
3 participants