-
Notifications
You must be signed in to change notification settings - Fork 54
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
apis for ip blocking and ip filtering #105
Conversation
Signed-off-by: Swapnil Wagh <waghswapnil@gmail.com>
imcsdk/apis/admin/ip.py
Outdated
handle.set_mo(mo) | ||
|
||
|
||
def is_ip_blocking_enabled(handle,): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+def is_ip_blocking_enabled(handle,): -> extra ','
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice catch. @waghswapnil how did vim not highlight this in red?
imcsdk/apis/admin/ip.py
Outdated
|
||
if mo.check_prop_match(**params): | ||
return (True, mo) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be directly **kwarags in check_prop_match instead of making dictionary.
handle.set_mo(mo) | ||
# Not returning the server copy as the connection drops after the above action | ||
return mo | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to add warning message as this will logout the existing user and not allow any other IP to login. Also applicable for modify case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning messages have been added already. please check the recent commit
LGTM [ fix this: def is_ip_blocking_enabled(handle,): ] |
Examples: | ||
ip_blocking_enable(handle, fail_count='6', | ||
fail_window='120', penalty_time='800') | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@waghswapnil this API is slightly confusing when it comes to param types..
the defaults specified are ints..
def ip_blocking_enable(handle, fail_count=5, fail_window=60, penalty_time=300, **kwargs):
the docstring says str
fail_count (str): Number of times a user can attempt to log in
+ unsuccessfully, before the system locks that user out
+ Range [3-10] attempts
+ fail_window (str): Length of time, in seconds, in which the
+ unsuccessful login attempts must occur in order
+ for the user to be locked out.
+ Range [60-120] seconds
+ penalty_time (str): The number of seconds the user remains locked out.
+ Range [300-900] seconds
the example specifies them as str
+ ip_blocking_enable(handle, fail_count='6',
+ fail_window='120', penalty_time='800')
when forming args internally we don't do str()
. We should clean this up for this and other APIs(if so).
The exists function also assumes str
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check comments
Signed-off-by: Swapnil Wagh <waghswapnil@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Swapnil Wagh waghswapnil@gmail.com