Skip to content

Siciarekm/s1 login#7

Merged
winderdoot merged 5 commits intodevelopfrom
siciarekm/S1_login
Mar 21, 2026
Merged

Siciarekm/s1 login#7
winderdoot merged 5 commits intodevelopfrom
siciarekm/S1_login

Conversation

@siciarmat
Copy link
Collaborator

@siciarmat siciarmat commented Mar 21, 2026

działa! przetestowane postmanem, prawidłowo zwraca JWT dla podanych danych uwzględnionych w bazie.
Przy ustawieniu testowym ClockSkew=TimeSpan.Zero (w rejestracji autoryzacji) [*] i timeout = 1 (w appsettings.json) dostęp do endpointu GET /api/v1/sessions/{id} jest przyznany faktycznie na minutę, później zwraca HTTP 401, zgodnie z przewidywaniami.

[*] przedtem trzeba było poczekać dłużej niż faktyczny timeout, zabezpieczenia dla spóźnialskich systemów, czy coś

@siciarmat siciarmat requested a review from winderdoot March 21, 2026 19:43
Copy link
Owner

@winderdoot winderdoot left a comment

Choose a reason for hiding this comment

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

Wolę przed wrzuceniem pr wykonać rebase na develop żeby obyło się bez merge commitów, ale może być.
Zaraz prejrzę zmiany i zatwierdzę. Przydałyby nam się jeszcze testy przed zamknięciem pierwszego sprintu. Może uda mi się coś w tym kierunku zrobić, ale jest słabo z czasem.

@winderdoot winderdoot self-requested a review March 21, 2026 21:22
Copy link
Owner

@winderdoot winderdoot left a comment

Choose a reason for hiding this comment

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

Tested and bug fixed. We should add unit tests when we have some more time.
LGTM

//return CreatedAtAction()
return CreatedAtAction(
actionName: endpoint,
routeValues: new { id = sessId },
Copy link
Owner

Choose a reason for hiding this comment

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

Wydaje mi się że tutaj nie powinniśmy wpisywać id = sessId. Z tym argumentem chodzi o to że on automatycznie jest potem wrzucany do uri zwracanego w headerze Location. Sprawdzałeś czy ten link zwracany w headerze będzie działał jeśli ustawia się id = sessId? Jeśli nie to na wszelki bym zamienił po prostu na new { sessId }

@winderdoot winderdoot merged commit b03f5e7 into develop Mar 21, 2026
1 check passed
@winderdoot winderdoot deleted the siciarekm/S1_login branch March 23, 2026 09:06
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.

2 participants