Add live authentication support in web app client #103
No reviewers
Labels
No labels
Compat/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Project/Client
Project/Server
Project/liblanbassador
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Blocks
Depends on
#99 Server option
require_auth is not implemented
luca0N/lanbassador
#104 Implement
require_auth option in server
luca0N/lanbassador
Reference
luca0N/lanbassador!103
Loading…
Reference in a new issue
No description provided.
Delete branch "dev_client_live_auth"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Authenticate with the session token as part of the live connection handshake, if the server indicates that authentication is required.
require_authis not implementedThis pull request depends on the functionality being implemented in the server. I have an early draft for this server code, but it's not ready to be committed yet.
7bd4c744d41e8f7a28acPrevious force-push from
7bd4c744d4to1e8f7a28acwas made to include missing changes. Those were amended into the previous commit.1e8f7a28ac809adb1bc2Force-push from
1e8f7a28acto809adb1bc2was made to fix a syntax error inAccountSession.ts.WIP: Add live authentication support in web app clientto Add live authentication support in web app client@ -31,4 +34,0 @@* @param token The session token to store.* @param rememberLogin If `true', then the session token is stored for* upwards of `SESSION_MAX_TIME' milliseconds, otherwise, the session token* can only be used during this session.This was erroneously removed.
require_authoption in serverUnit tests must be added for the new functionality before merging this pull request, particularly for the
LiveConnectionclass, and theAccountSessionclass needs some special attention (see related comment).@ -70,0 +101,4 @@resolve();});});}There's no
.specfile for theAccountSessionclass at this time, and adding a unit test for it would be slightly tricky because it modifies browser cookies directly. Thus, there are two ways to create unit testing for this class, as I see it:AccountSessionthat mocksloadSessionToken().This must be evaluated before merging.
Commit
49a5487997introduces theAccountSessionMockclass that allows mocking session tokens. However, it still wouldn't make sense to create unit testing for the functions forAccountSessionalone; the new mock class only helps with other unit testing sources.As such, unit testing for AccountSession will not be implemented.
@ -297,0 +339,4 @@return;}if (packet.action === PacketAction.RET_AUTHENTICATION_REQUIRED) {A unit test must be created for testing the client behavior when
RET_AUTHENTICATION_REQUIREDis received.@ -297,0 +351,4 @@.withData({ session_token: sessionToken }).build();this.loggingIn = true;this.ws.send(JSON.stringify(packet));A unit test must be created for testing the
LOGINpacket action.Added by commit
49a5487997.Marking this pull request as a WIP once again (see comments in the previous code review).
@ -105,0 +120,4 @@wsMock.mockDataIncoming(JSON.stringify({ action: "QUERY_HISTORY" }));wsMock.mockDataOutgoing(JSON.stringify({ action: "RET_AUTHENTICATION_REQUIRED", data: "" }));wsMock.mockDataIncoming(JSON.stringify({ action: "LOGIN", data: { session_token: mockedSessionToken } }));I have had trouble mocking anything beyond this packet, for some reason. I don't have enough time to solve this at this time, but I will do some more investigation once I have some time.
Fixed by
008e3298fd.49a54879977d5d4cecd2Previous force-push operation from
49a5487997to7d5d4cecd2was made to solve multiple conflicts on multiple files that were introduced after merging !105.7d5d4cecd2bd532c03a0Previous force-push operation from
7d5d4cecd2tobd532c03a0was made to fix a build error that was introduced after rebasing this branch withdev, and to remove a trailing space in a line fromAccountSessionMock.ts.A few issues were introduced after rebasing with dev.
@ -19,6 +19,8 @@import { Injectable, } from "@angular/core";import { ServerComms } from "../ServerComms";Missing trailing comma (see related comment).
Fixed; see #103 (comment).
@ -0,0 +20,4 @@import { Injectable } from '@angular/core';import { ServerComms } from "../ServerComms";import { AccountSessionProvider, AccountSession } from "./AccountSession";Missing trailing comma; this violates the project coding conventions. See !105.
Fixed; see #103 (comment).
@ -1,3 +1,4 @@This line break should not be in here.
Fixed; see #103 (comment).
bd532c03a0a7cdd49d22Previous force-push operation from
bd532c03a0toa7cdd49d22was made to fix previous review comments1 2 3.#103/files (comment) ↩︎
#103/files (comment) ↩︎
#103/files (comment) ↩︎
a7cdd49d22e0cd39a034Previous force-push operation from
a7cdd49d22toe0cd39a034introduced a minor change in the packet builder class to fix a bug. The bug caused client packets to always have an undefineddatafield.6fc165739a008e3298fdPrevious force-push operation from
6fc165739ato008e3298fdwas made to include changes that were left off in the messages files.View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.Merge
Merge the changes and update on Forgejo.