PAMv3 implementation #122
PAMv3 implementation #122
Conversation
Added: * grant_token method * parse_token method * cassettes for VCR tests * contract tests * contract tests integration with github actions Co-authored with: Łukasz Klich ;)
|
@parfeon few questions:
|
lib/pubnub/events/grant_token.rb
Outdated
| @event = current_operation | ||
| @uuid_perms = options[:uuids] || {} | ||
| @channel_perms = options[:channels] || {} | ||
| @channel_group_perms = options[:channel_groups] || {} |
There was a problem hiding this comment.
@kleewho it will be great to add new instance variables into create_variables_from_options method variables words list.
There was a problem hiding this comment.
yeah, sure, I'll add it. Although I'm wondering what's the advantage? Could you elaborate?
There was a problem hiding this comment.
@kleewho it is basically "built-in" in event automated variable assignment from passed list of options:
variables.each { |variable| instance_variable_set('@' + variable, options[variable.to_sym]) unless variable.nil? }You can see, that instance_variable_set maps variables to values in options.
There was a problem hiding this comment.
Yeaaaaaah. How it will work with my name change though? I had to change it because event expect something else from channels than we have in grant_token
There was a problem hiding this comment.
@kleewho I've totally forgot about formatter, which tries to pull out list of channels from array or string with comma-separated channel / groups names.
With this in mind it makes sense to clear those lists before super class will try to process them.
Other option can be use different names for arguments. Let user pass values under channel_perms - but not necessary.
|
FYI Plugging in acceptance tests is not yet finished |
`Pam` was getting in to the way more often than it was helping
|
@parfeon I've run the acceptance tests against pubnub servers and against the mock. There's also better implementation of validation. |
There was a problem hiding this comment.
LGTM. I will let @parfeon approve the PR, if his comments were addressed.
| - name: Expose main report | ||
| uses: actions/upload-artifact@v2 | ||
| if: always() | ||
| with: | ||
| name: main.xml | ||
| path: main.xml | ||
| - name: Expose beta report | ||
| uses: actions/upload-artifact@v2 | ||
| if: always() | ||
| with: | ||
| name: beta.xml | ||
| path: beta.xml |
There was a problem hiding this comment.
@kleewho it will be better to upload two files as part of single artifact to reduce amount of customisations, which will be required for each repository to process reports.
Dart fo example upload both reports as acceptance-test-reports.
Maybe we can make it easier by modification of Combine test results task:
- name: Combine test results
if: always()
run: |
mkdir ./reports
sudo npm install -g junit-report-merger &&
jrm ./reports/main.xml "./main/**/*.xml" &&
jrm ./reports/beta.xml "./beta/**/*.xml"and then create single task to upload both reports:
- name: Upload acceptance tests reports
uses: actions/upload-artifact@v2
if: always()
with:
name: acceptance-test-reports
path: ./reports/*.xml
retention-days: 7With retention-days we also limit for how long reports will be stored. As soon as analyser will pull them out, there will be no use from the. With this configuration artifacts will be invalidated in 7 days.
There was a problem hiding this comment.
Sure. Not a problem for me. I wasn't sure what's the expected way to do it
lib/pubnub/events/grant_token.rb
Outdated
| if @read | ||
| sum |= 1 | ||
| end | ||
|
|
||
| if @write | ||
| sum |= 2 | ||
| end | ||
|
|
||
| if @manage | ||
| sum |= 4 | ||
| end | ||
|
|
||
| if @delete | ||
| sum |= 8 | ||
| end | ||
|
|
||
| if @create | ||
| sum |= 16 | ||
| end | ||
|
|
||
| if @get | ||
| sum |= 32 | ||
| end | ||
|
|
||
| if @update | ||
| sum |= 64 | ||
| end | ||
|
|
||
| if @join | ||
| sum |= 128 | ||
| end |
There was a problem hiding this comment.
@kleewho all these can be much smaller:
sum |= 1 if @read
sum |= 2 if @write
sum |= 4 if @manage
sum |= 8 if @delete
sum |= 16 if @create
sum |= 32 if @get
sum |= 64 if @update
sum |= 128 if @joinAnd pretty easy to read :)
|
@client-engineering-bot release |
Fix version format written with scripts by mistake (wrong settings in script).
4 participants
feat: Method grant_token has been added
grant_token allows generation of signed token with permissions for channels, channel groups and uuids