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

More precise server user control #13015

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public class AuthorizedUserRepository {

private static final AuthorizedUserRepository instance;
static {
instance = new AuthorizedUserRepository(DatabaseUtils.prepareH2Connection(DatabaseUtils.DB_NAME_USERS, false));
instance = new AuthorizedUserRepository(DatabaseUtils.prepareSqliteConnection(DatabaseUtils.DB_NAME_USERS));
}

private Dao<AuthorizedUser, Object> usersDao;
Expand All @@ -42,6 +42,7 @@ public AuthorizedUserRepository(String connectionString) {
if (!file.exists()) {
file.mkdirs();
}

try {
ConnectionSource connectionSource = new JdbcConnectionSource(connectionString);
TableUtils.createTableIfNotExists(connectionSource, AuthorizedUser.class);
Expand Down
4 changes: 2 additions & 2 deletions Mage.Server/src/main/java/mage/server/MageServerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ static private String generateAuthToken() {

@Override
public boolean authSendTokenToEmail(String sessionId, String email) throws MageException {
if (!managerFactory.configSettings().isAuthenticationActivated()) {
if (!managerFactory.configSettings().isRegistrationEnabled()) {
sendErrorMessageToClient(sessionId, Session.REGISTRATION_DISABLED_MESSAGE);
return false;
}
Expand Down Expand Up @@ -115,7 +115,7 @@ public boolean authSendTokenToEmail(String sessionId, String email) throws MageE

@Override
public boolean authResetPassword(String sessionId, String email, String authToken, String password) throws MageException {
if (!managerFactory.configSettings().isAuthenticationActivated()) {
if (!managerFactory.configSettings().isRegistrationEnabled()) {
sendErrorMessageToClient(sessionId, Session.REGISTRATION_DISABLED_MESSAGE);
return false;
}
Expand Down
6 changes: 3 additions & 3 deletions Mage.Server/src/main/java/mage/server/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ public static void main(String[] args) {
final ConfigWrapper config = new ConfigWrapper(ConfigFactory.loadFromFile(configPath));


if (config.isAuthenticationActivated()) {
if (config.isRegistrationEnabled() || config.shouldCheckUsers()) {
logger.info("Check authorized user DB version ...");
if (!AuthorizedUserRepository.getInstance().checkAlterAndMigrateAuthorizedUser()) {
logger.fatal("Failed to start server.");
Expand Down Expand Up @@ -260,8 +260,8 @@ public static void main(String[] args) {
logger.info("Config - max pool size : " + config.getMaxPoolSize());
logger.info("Config - num accp.threads: " + config.getNumAcceptThreads());
logger.info("Config - second.bind port: " + config.getSecondaryBindPort());
logger.info("Config - users registr.: " + (config.isAuthenticationActivated() ? "true" : "false"));
logger.info("Config - users anon: " + (!config.isAuthenticationActivated() ? "true" : "false"));
logger.info("Config - reg. enabled : " + (config.isRegistrationEnabled() ? "true" : "false"));
logger.info("Config - check users : " + (config.shouldCheckUsers() ? "true" : "false"));
logger.info("Config - mailgun api key : " + config.getMailgunApiKey());
logger.info("Config - mailgun domain : " + config.getMailgunDomain());
logger.info("Config - mail smtp Host : " + config.getMailSmtpHost());
Expand Down
8 changes: 4 additions & 4 deletions Mage.Server/src/main/java/mage/server/Session.java
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public Session(ManagerFactory managerFactory, String sessionId, InvokerCallbackH
}

public String registerUser(String userName, String password, String email) {
if (!managerFactory.configSettings().isAuthenticationActivated()) {
if (!managerFactory.configSettings().isRegistrationEnabled()) {
String returnMessage = REGISTRATION_DISABLED_MESSAGE;
sendErrorMessageToClient(returnMessage);
return returnMessage;
Expand Down Expand Up @@ -242,7 +242,7 @@ public String connectUserHandling(String userName, String password, String resto

// find auth user
AuthorizedUser authorizedUser = null;
if (managerFactory.configSettings().isAuthenticationActivated()) {
if (managerFactory.configSettings().shouldCheckUsers()) {
authorizedUser = AuthorizedUserRepository.getInstance().getByName(userName);
String errorMsg = "Wrong username or password. You must register your account first.";
if (authorizedUser == null) {
Expand Down Expand Up @@ -276,8 +276,8 @@ public String connectUserHandling(String userName, String password, String resto
if (newUser == null) {
User anotherUser = managerFactory.userManager().getUserByName(userName).orElse(null);
if (anotherUser != null) {
boolean canDisconnectAuthDueAnotherInstance = managerFactory.configSettings().isAuthenticationActivated();
boolean canDisconnectAnonDueSameHost = !managerFactory.configSettings().isAuthenticationActivated()
boolean canDisconnectAuthDueAnotherInstance = managerFactory.configSettings().shouldCheckUsers();
boolean canDisconnectAnonDueSameHost = !managerFactory.configSettings().shouldCheckUsers()
Copy link
Member

@JayDi85 JayDi85 Oct 20, 2024

Choose a reason for hiding this comment

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

I don’t test it but disconnect another instance of the user is very important for xmage game engine (it can work with single user’s instance only). There are must be 1 user per player (registered or anon — each mode must keep single user). So game will see only 1 user to send and wait feedback.

On login logic:

  • in registered mode: disconnect all other user instances;
  • in anon mode: disconnect all other user instances if it’s same host (ip address).it’s important to restrict login with same name but diff IP (e.g. you can’t disconnect another logged user).

So user instance consistency must be enabled all the time. No need to setup it.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for pointing that out!
I will do some testing on my own as well.

&& ANON_IDENTIFY_BY_HOST
&& Objects.equals(anotherUser.getHost(), host);
boolean canDisconnectAnyDueSessionRestore = Objects.equals(restoreSessionId, anotherUser.getRestoreSessionId());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public interface ConfigSettings {

Boolean isSaveGameActivated();

Boolean isAuthenticationActivated();
Boolean isRegistrationEnabled();

String getGoogleAccount();

Expand All @@ -69,4 +69,6 @@ public interface ConfigSettings {
List<Plugin> getDraftCubes();

List<Plugin> getDeckTypes();

Boolean shouldCheckUsers();
}
1 change: 1 addition & 0 deletions Mage.Server/src/main/java/mage/server/util/Config.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
<xs:attribute name="userNamePattern" type="xs:string" use="required"/>
<xs:attribute name="maxAiOpponents" type="xs:string" use="optional"/>
<xs:attribute name="saveGameActivated" type="xs:boolean" use="optional"/>

</xs:complexType>
</xs:element>

Expand Down
8 changes: 6 additions & 2 deletions Mage.Server/src/main/java/mage/server/util/ConfigWrapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,12 @@ public Boolean isSaveGameActivated() {
return config.getServer().isSaveGameActivated();
}

public Boolean isAuthenticationActivated() {
return config.getServer().isAuthenticationActivated();
public Boolean isRegistrationEnabled() {
return config.getServer().isRegistrationEnabled();
}

public Boolean shouldCheckUsers() {
return config.getServer().isCheckUsers();
}

public String getGoogleAccount() {
Expand Down
3 changes: 2 additions & 1 deletion Mage.Server/src/main/xml-resources/jaxb/Config/Config.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
<xs:attribute name="maxPasswordLength" type="xs:positiveInteger" use="required"/>
<xs:attribute name="maxAiOpponents" type="xs:string" use="optional"/>
<xs:attribute name="saveGameActivated" type="xs:boolean" use="optional"/>
<xs:attribute name="authenticationActivated" type="xs:boolean" use="optional"/>
<xs:attribute name="googleAccount" type="xs:string" use="optional"/>
<xs:attribute name="mailgunApiKey" type="xs:string" use="optional"/>
<xs:attribute name="mailgunDomain" type="xs:string" use="optional"/>
Expand All @@ -44,6 +43,8 @@
<xs:attribute name="mailUser" type="xs:string" use="optional"/>
<xs:attribute name="mailPassword" type="xs:string" use="optional"/>
<xs:attribute name="mailFromAddress" type="xs:string" use="optional"/>
<xs:attribute name="registrationEnabled" type="xs:boolean" use="required"/>
<xs:attribute name="checkUsers" type="xs:boolean" use="required"/>
</xs:complexType>
</xs:element>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ static class ConfigBuilder extends FluentBuilder<Config, ConfigBuilder> {
public int maxPasswordLength;
public String maxAiOpponents;
public boolean saveGameActivated;
public boolean authenticationActivated;
public boolean shouldCheckUsers;
public boolean registrationEnabled;
public String googleAccount;
public String mailgunApiKey;
public String mailgunDomain;
Expand Down Expand Up @@ -94,7 +95,8 @@ private Server makeServer() {
server.setMaxPasswordLength(bi(maxPasswordLength));
server.setMaxAiOpponents(maxAiOpponents);
server.setSaveGameActivated(saveGameActivated);
server.setAuthenticationActivated(authenticationActivated);
server.setRegistrationEnabled(registrationEnabled);
server.setCheckUsers(shouldCheckUsers);
server.setGoogleAccount(googleAccount);
server.setMailgunApiKey(mailgunApiKey);
server.setMailgunDomain(mailgunDomain);
Expand Down Expand Up @@ -171,7 +173,8 @@ Stream<DynamicTest> assignmentFromServer() {
testInt("max password length", c -> c.maxPasswordLength = expectedPositiveInt, ConfigWrapper::getMaxPasswordLength),
testString("max AI opponents", c -> c.maxAiOpponents = expectedString, ConfigWrapper::getMaxAiOpponents),
testTrue("save game activated", c -> c.saveGameActivated = true, ConfigWrapper::isSaveGameActivated),
testTrue("authentication activated", c -> c.authenticationActivated = true, ConfigWrapper::isAuthenticationActivated),
testTrue("registration enabled", c -> c.registrationEnabled = true, ConfigWrapper::isRegistrationEnabled),
testTrue("should check users", c -> c.shouldCheckUsers = true, ConfigWrapper::shouldCheckUsers),
testString("google account", c -> c.googleAccount = expectedString, ConfigWrapper::getGoogleAccount),
testString("mailgun api key", c -> c.mailgunApiKey = expectedString, ConfigWrapper::getMailgunApiKey),
testString("mailgun domain", c -> c.mailgunDomain = expectedString, ConfigWrapper::getMailgunDomain),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public class DatabaseUtils {
// warning, do not change names or db format
// h2
public static final String DB_NAME_FEEDBACK = "feedback.h2";
public static final String DB_NAME_USERS = "authorized_user.h2";
public static final String DB_NAME_USERS = "authorized_users.db";
public static final String DB_NAME_CARDS = "cards.h2";
// sqlite (usage reason: h2 database works bad with 1GB+ files and can break it)
public static final String DB_NAME_RECORDS = "table_record.db";
Expand Down
Loading