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

fix saving signed_tokens field when room is permanent #2843

Merged
merged 1 commit into from
Jan 10, 2022

Conversation

timsolov
Copy link
Contributor

@timsolov timsolov commented Dec 28, 2021

#2825
fix saving room's configuration to config file when room is permanent.

@lminiero
Copy link
Member

lminiero commented Jan 3, 2022

This patch goes beyond just saving the property to the config file, when saving rooms permanently. It also adds an option to "edit" to change the behaviour dynamically within a room. Please only do the former in this patch: the latter is something that is not there yet, and I'm not sure it should be, and so needs a separate discussion.

@timsolov
Copy link
Contributor Author

timsolov commented Jan 8, 2022

@lminiero updated

@@ -3445,6 +3445,8 @@ static json_t *janus_videoroom_process_synchronous_request(janus_videoroom_sessi
janus_config_add(config, c, janus_config_item_create("is_private", "yes"));
if(videoroom->require_pvtid)
janus_config_add(config, c, janus_config_item_create("require_pvtid", "yes"));
if(videoroom->signed_tokens)
janus_config_add(config, c, janus_config_item_create("signed_tokens", "yes"));
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be in the "edit" part too? Even if "edit" doesn't expose a way to modify that property, if you don't have lines to save the value there, a permanent "edit" will result in not saving the signed_tokens property, which would be lost as a consequence.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeh, you're right. I forgot about edit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lminiero done

Copy link
Member

@lminiero lminiero left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fixes! But I see some new lines were added that weren't there before: please only keep the permanent saving of the property.

@@ -2328,11 +2328,14 @@ int janus_videoroom_init(janus_callbacks *callback, const char *config_path) {
videoroom->is_private = priv && priv->value && janus_is_true(priv->value);
videoroom->require_pvtid = req_pvtid && req_pvtid->value && janus_is_true(req_pvtid->value);
if(signed_tokens && signed_tokens->value && janus_is_true(signed_tokens->value)) {
JANUS_LOG(LOG_INFO, "Room id %s signed_tokens value: %s\n", videoroom->room_id_str, signed_tokens->value);
Copy link
Member

Choose a reason for hiding this comment

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

This is new, please remove.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

if(!gateway->auth_is_signed()) {
JANUS_LOG(LOG_WARN, "Can't enforce signed tokens for this room, signed-mode not in use in the core\n");
} else {
videoroom->signed_tokens = TRUE;
}
} else {
JANUS_LOG(LOG_INFO, "Room id %s signed_tokens value: false\n", videoroom->room_id_str);
Copy link
Member

Choose a reason for hiding this comment

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

Please remove.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

@@ -3005,6 +3008,7 @@ static int janus_videoroom_access_room(json_t *root, gboolean check_modify, gboo
}
}
if(check_join) {
JANUS_LOG(LOG_INFO, "check_join = true (%s)\n", room_id_str);
Copy link
Member

Choose a reason for hiding this comment

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

This looks like a debug line that was left there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

plugins/janus_videoroom.c Outdated Show resolved Hide resolved
@lminiero
Copy link
Member

Thanks, merging then!

@lminiero lminiero merged commit 23b7c21 into meetecho:master Jan 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants