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

feat(authz-keycloak): add token refresh/expire time configuration #6229

Merged

Conversation

Belyenochi
Copy link
Contributor

@Belyenochi Belyenochi commented Jan 28, 2022

resolved #3479

@bisakhmondal bisakhmondal changed the title feat:support configurating token refresh/expire time docs(authz-keycloak): add token refresh/expire time configuration Jan 30, 2022
Copy link
Member

@spacewander spacewander left a comment

Choose a reason for hiding this comment

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

No feature added?

@tzssangglass
Copy link
Member

No feature added?

The code has been implemented, but not covered by the documentation and test cases,

-- Return access_token expires_in value (in seconds).
local function authz_keycloak_access_token_expires_in(conf, expires_in)
return (expires_in or conf.access_token_expires_in or 300)
- 1 - (conf.access_token_expires_leeway or 0)
end
-- Return refresh_token expires_in value (in seconds).
local function authz_keycloak_refresh_token_expires_in(conf, expires_in)
return (expires_in or conf.refresh_token_expires_in or 3600)
- 1 - (conf.refresh_token_expires_leeway or 0)
end

@tzssangglass
Copy link
Member

pls add description of access_token_expires_in and refresh_token_expires_in

@Belyenochi
Copy link
Contributor Author

pls add description of access_token_expires_in and refresh_token_expires_in

I will add the test case

Copy link
Member

@spacewander spacewander left a comment

Choose a reason for hiding this comment

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

Need to add these fields in

@Belyenochi
Copy link
Contributor Author

Need to add these fields in

Thanks for the advice

@tzssangglass
Copy link
Member

The code has been implemented, but not covered by the documentation and test cases,

In fact, the description of these two parameters is also missing

@Belyenochi
Copy link
Contributor Author

The code has been implemented, but not covered by the documentation and test cases,

In fact, the description of these two parameters is also missing

I have tried adding test cases and descriptions.

@Belyenochi
Copy link
Contributor Author

Belyenochi commented Feb 22, 2022

@tzssangglass The fix has been completed, thanks for review.

@tzssangglass
Copy link
Member

hi @Belyenochi , thank you for contributing, pls fix Doc Lint check,just a little problem that needs to be fixed.

apisix/plugins/authz-keycloak.lua Show resolved Hide resolved
docs/en/latest/plugins/authz-keycloak.md Outdated Show resolved Hide resolved
apisix/plugins/authz-keycloak.lua Show resolved Hide resolved
@spacewander
Copy link
Member

hi @Belyenochi , thank you for contributing, pls fix Doc Lint check,just a little problem that needs to be fixed.

Solved in 1c6b473

spacewander
spacewander previously approved these changes Feb 23, 2022
apisix/plugins/authz-keycloak.lua Outdated Show resolved Hide resolved
@spacewander spacewander changed the title docs(authz-keycloak): add token refresh/expire time configuration feat(authz-keycloak): add token refresh/expire time configuration Feb 23, 2022
@Belyenochi
Copy link
Contributor Author

@tzssangglass hi tzssangglass, pls help to review and merge this PR.

@spacewander spacewander merged commit 4b4e727 into apache:master Feb 23, 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.

authz-keycloak: support configurating token refresh/expire time
3 participants