-
Notifications
You must be signed in to change notification settings - Fork 65
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
Install admin accnt (was Auth12 on non merged repo) #6
Conversation
… CORAL admin. Removed unreachable echo from ajax_processing.php
} else if (!$db->query($create_admin_query)) { | ||
$errorMessage[] = "Failed to create CORAL Admin"; | ||
//passed db host, name check, test that user can select from Auth database | ||
} else if (!$db->query("SELECT loginID FROM $database->dbname.User WHERE loginID like '%$admin->coral_username%';")){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is hard to read. Could the variables be separated from the query string to improve readability? The result would be something like this:
...$db->query("SELECT loginID FROM " . $database->dbname . ".User WHERE loginID like '%" . $admin->coral_username . "%';")...
Besides my one suggested change for code readability, these changes all look good from a code perspective. But I haven't tested this code, and I think it deserves being tested again before merging it. @jeffnm I think these improvements will help the proposed Combined Installer project. |
I've made some improvements to readability. The first was @remocrevo's suggestion to break out the variables in a query string. The other was to separate the DB username form and the CORAL admin username form visually. I did the same for the LDAP section. I ran two test installs.
Both succeeded and allowed me to login to the auth module with them. |
PHP Fatal error: Uncaught Error: Call to a member function set_charset() on boolean in …/classes/DBService.php:49 Stack trace: #0 …/classes/DBService.php(28): DBService->connect() #1 …/classes/BaseObject.php(12): DBService->init(Object(NamedArguments)) #2 …/classes/DBService.php(19): BaseObject->__construct() #3 …/classes/DatabaseObject.php(40): DBService::getInstance() #4 …/classes/BaseObject.php(12): DatabaseObject->init(Object(NamedArguments)) #5 …/auth/ajax_htmldata.php(11): BaseObject->__construct() coral-erm#6 {main} thrown in …/classes/DBService.php on line 49, referer: http://…/auth/admin.php
…d-sites-3.0.1-compat ERM-59: Cannot remove Authorized Sites from Access information in Resource records
The main purpose of this pull request is to add a requested feature: specifying coral admin account during the auth install process.
The secondary (side-effect) purpose of this pull request is to cleanup the install script to make it easier to read and understand at a glance.
These changes work fine on my development workstation.