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

Added access control public header file. #1300

Merged

Conversation

ethouris
Copy link
Collaborator

This header file contains now only the predefined rejection codes.

It's also intended to be expanded in future to contain also function headers for utilities that allow to parse the StreamID according to the access control standard syntax.

@ethouris ethouris added Priority: High Type: Enhancement Indicates new feature requests [core] Area: Changes in SRT library core Impact: Optional labels May 22, 2020
@ethouris ethouris added this to the v1.5.0 milestone May 22, 2020
@ethouris ethouris self-assigned this May 22, 2020
@maxsharabayko maxsharabayko mentioned this pull request May 22, 2020
33 tasks
Copy link
Collaborator

@maxsharabayko maxsharabayko left a comment

Choose a reason for hiding this comment

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

Probably AccessControl.md and API.md should be updated to reference this header.

srtcore/access_control.h Outdated Show resolved Hide resolved
ethouris and others added 2 commits May 25, 2020 15:04
Co-authored-by: Maxim Sharabayko <maxlovic@gmail.com>
Comment on lines 131 to 133
Codes from this range can be only understood if both applications know the
code definitions of the other, and these should be used only after making
sure that both applications understood these codes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Codes from this range can be only understood if both applications know the
code definitions of the other, and these should be used only after making
sure that both applications understood these codes.
Codes from this range can be only understood if each applications knows the
code definitions of the other. These codes should be used only after making
sure that both applications understood them.

Comment on lines 135 to 136
The intention for the predefined codes is to be consistent with the HTTP
standard codes, therefore there are following sub-ranges used:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
The intention for the predefined codes is to be consistent with the HTTP
standard codes, therefore there are following sub-ranges used:
The intention for the predefined codes is to be consistent with HTTP
standard codes. Therefore the following sub-ranges are used:

* 400 - 599: Client and server errors in HTTP, adopted by SRT
* 600 - 999: unused in SRT

This code can be set by using the `srt_setrejectreason` function.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
This code can be set by using the `srt_setrejectreason` function.
Such a code can be set by using the `srt_setrejectreason` function.

Comment on lines 149 to 152
This code should be set by the callback handler in the beginning in case
when the application needs to be informed that the callback handler
actually has interpreted the incoming connection, but it didn't set any
more appropriate code describing the situation.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
This code should be set by the callback handler in the beginning in case
when the application needs to be informed that the callback handler
actually has interpreted the incoming connection, but it didn't set any
more appropriate code describing the situation.
This code should be set by the callback handler in the beginning in case
the application needs to be informed that the callback handler
actually has interpreted the incoming connection, but hasn't set a
more appropriate code describing the situation.

Authentication failed, which makes the client unauthorized to access the
resource. This error, however, confirms that the syntax is correct and
the resource has been properly identified. Note that this cannot be
reported in case when you use a simple user-password authentication
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
reported in case when you use a simple user-password authentication
reported when you use a simple user-password authentication

Comment on lines 242 to 246
The resource specified in the `r` key (with combination of `h` key)
is not found at this time. This error should be only reported if the
information about resource accessibility is allowed to be publicly
visible, otherwise the application might rather report authorization
errors.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
The resource specified in the `r` key (with combination of `h` key)
is not found at this time. This error should be only reported if the
information about resource accessibility is allowed to be publicly
visible, otherwise the application might rather report authorization
errors.
The resource specified in the `r` key (in combination with the `h` key)
is not found at this time. This error should be only reported if the
information about resource accessibility is allowed to be publicly
visible. Otherwise the application might report authorization
errors.

### SRT_REJX_CONFLICT

The resource being accessed is already locked for modification. This error
should only be reported for `m=publish` in case when the resource being
Copy link
Collaborator

@stevomatthews stevomatthews May 29, 2020

Choose a reason for hiding this comment

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

Suggested change
should only be reported for `m=publish` in case when the resource being
should only be reported for `m=publish` when the resource being


### SRT_REJX_UNIMPLEMENTED

The request was correctly recognized, but the current version doesn't
Copy link
Collaborator

Choose a reason for hiding this comment

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

"current version" of what? SRT? The application? Both? Later you refer to a "server version". How does this relate to SRT and/or the application?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's all about the server version. The rejection codes concern exclusively a situation when you have a service hanging as "SRT Listener" and a client tries to connect, with having a query specified in the StreamID field. Although yes, it should be clarified that it's about the version of the server application (can be both SRT or any other part of the service software).

Comment on lines 333 to 335
The service is down for maintenance. This can only be reported in
case when the servis has been temporarily replaced by a stub that is only
reporting this error, while the real service is down for maintenance.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
The service is down for maintenance. This can only be reported in
case when the servis has been temporarily replaced by a stub that is only
reporting this error, while the real service is down for maintenance.
The service is down for maintenance. This can only be reported
when the service has been temporarily replaced by a stub that is only
reporting this error, while the real service is down for maintenance.

@maxsharabayko maxsharabayko modified the milestones: v1.5.0, v1.5.0 - Sprint 16 Jun 4, 2020
@mbakholdina mbakholdina modified the milestones: v1.5.0 - Sprint 16, v1.5.0 - Sprint 17 Jun 15, 2020
Copy link
Collaborator

@maxsharabayko maxsharabayko left a comment

Choose a reason for hiding this comment

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

Thought. Might be that all those rejections codes should go to API-functions.md, while AcessControl.md should only have a list and a reference to API-functions.md.

docs/AccessControl.md Outdated Show resolved Hide resolved
docs/AccessControl.md Outdated Show resolved Hide resolved
@maxsharabayko maxsharabayko merged commit 47fe4eb into Haivision:master Jun 23, 2020
@alexpokotilo
Copy link
Contributor

what api should we use to set these rejection reasons ?

@ethouris
Copy link
Collaborator Author

See srt_setrejectreason and srt_getrejectreason.

@mbakholdina mbakholdina modified the milestones: v1.5.0 - Sprint 17, v1.4.2 Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[core] Area: Changes in SRT library core Priority: High Type: Enhancement Indicates new feature requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants