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

WIP: Refresh session param for whoami endpoint, PLATFORM-6607 #48

Merged
merged 18 commits into from
Jan 19, 2022

Conversation

abador
Copy link

@abador abador commented Dec 29, 2021

Add a way to refresh a currently used session in the whoami endpoint.
Issue tracked in upstream: #615

Admin part is based on this pr: #2011

// Redirect to public endpoint
admin.Handle(m, RouteWhoami, x.RedirectToPublicRoute(h.r))
}
admin.DELETE(RouteIdentityManagement, h.deleteIdentitySessions)
admin.PATCH(RouteSession, h.adminSessionRefresh)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't it be more sensible to have session refresh only on PATCH method? GET should be non-mutable.

Copy link
Author

Choose a reason for hiding this comment

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

I missed that sorry

session/handler.go Outdated Show resolved Hide resolved
session/handler.go Outdated Show resolved Hide resolved
session/session.go Show resolved Hide resolved
session/session.go Show resolved Hide resolved
session/handler.go Show resolved Hide resolved
Copy link

@zepatrik zepatrik left a comment

Choose a reason for hiding this comment

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

The upstream PR got merged btw 😉

@harnash harnash force-pushed the PLATFORM-6607-prolong-session branch from 92383e9 to 85a92a1 Compare January 11, 2022 18:05
@harnash harnash force-pushed the PLATFORM-6607-prolong-session branch from 85a92a1 to 9479410 Compare January 11, 2022 19:09
session/handler.go Outdated Show resolved Hide resolved
@abador
Copy link
Author

abador commented Jan 12, 2022

@zepatrik @aeneasr @Benehiko @arekkas can you take a look?

@aeneasr
Copy link

aeneasr commented Jan 12, 2022

Can you make a PR against our repo? :)

@aeneasr
Copy link

aeneasr commented Jan 12, 2022

Ah got it, will take a look to confirm

Copy link

@aeneasr aeneasr left a comment

Choose a reason for hiding this comment

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

I think this looks really good! Just needs some more tests :)

driver/config/config.go Outdated Show resolved Hide resolved
@@ -178,6 +183,15 @@ func (h *Handler) whoami(w http.ResponseWriter, r *http.Request, ps httprouter.P
return
}

// Refresh session if param was true
refresh := r.URL.Query().Get("refresh")
Copy link

Choose a reason for hiding this comment

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

Add this to the OpenAPI spec :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fix in b69eb97

embedx/config.schema.json Outdated Show resolved Hide resolved
session/session.go Outdated Show resolved Hide resolved
internal/httpclient/README.md Outdated Show resolved Hide resolved
- oryAccessToken: []
summary: |-
Calling this endpoint refreshes a current user session.
If `session.refresh_time_window` is set it will only refresh the session after this time has passed.

Choose a reason for hiding this comment

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

Suggested change
If `session.refresh_time_window` is set it will only refresh the session after this time has passed.
If `session.refresh_min_time_left` is set it will only refresh the session after this time has passed.

internal/httpclient/api/openapi.yaml Outdated Show resolved Hide resolved
internal/httpclient/api/openapi.yaml Outdated Show resolved Hide resolved
internal/httpclient/api/openapi.yaml Outdated Show resolved Hide resolved
session/handler.go Outdated Show resolved Hide resolved
session/handler.go Outdated Show resolved Hide resolved
session/handler.go Outdated Show resolved Hide resolved
session/handler.go Outdated Show resolved Hide resolved
session/handler.go Outdated Show resolved Hide resolved
@abador abador force-pushed the PLATFORM-6607-prolong-session branch from 3be64f1 to 0caadac Compare January 18, 2022 19:47
@mmeller-wikia mmeller-wikia merged commit f1f8740 into fandom_master Jan 19, 2022
@mmeller-wikia mmeller-wikia deleted the PLATFORM-6607-prolong-session branch January 19, 2022 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants